Bug report #8818

"value map" widget broken

Added by Giovanni Manghi over 10 years ago. Updated over 9 years ago.

Status:Closed
Priority:Severe/Regression
Assignee:-
Category:Vectors
Affected QGIS version:master Regression?:No
Operating System: Easy fix?:No
Pull Request or Patch supplied:Yes Resolution:wontfix
Crashes QGIS or corrupts data:No Copied to github as #:17511

Description

The "value map" widget always write the "description" field instead of the "value" one as expected.

Is a regression since 1.8.

valuemap.zip (5.6 KB) Giovanni Manghi, 2013-10-10 07:38 AM

valuemap.mp4 (4.6 MB) Giovanni Manghi, 2013-12-18 07:49 AM

Associated revisions

Revision 48427e18
Added by Jürgen Fischer about 10 years ago

identify results: fix value map updates (fixes #8818)
dual view: avoid empty undo commands

History

#1 Updated by Matthias Kuhn over 10 years ago

Does this also happen in master? I just tested it there and this does not happen for me.

#2 Updated by Giovanni Manghi over 10 years ago

on 2.1.0+git20131010+f49ea36~precise-ubuntugis1 does not work, is the fix newer than this revision?

#3 Updated by Matthias Kuhn over 10 years ago

I don't know of any fix. I was just surprised because it seemed to work for me today. Or maybe I'm doing it wrong?

#4 Updated by Giovanni Manghi over 10 years ago

The widget should allow the user to choose by what is defined in the "description" column, but then write what is defined in the "value" column.

#5 Updated by Matthias Kuhn over 10 years ago

Works here.
As soon as I switch back to LineEdit, the value gets shown again, wherever the description has been shown before.

#6 Updated by Giovanni Manghi over 10 years ago

Matthias Kuhn wrote:

Works here.
As soon as I switch back to LineEdit, the value gets shown again, wherever the description has been shown before.

try the attached project and edit the values of the column "landuse" from the table of attributes. It should allow to choose the common tree name and then write the scientific name.

#7 Updated by Matthias Kuhn over 10 years ago

You are right Giovanni.
Maybe in my previous tests it didn't appear, because I was using an integer as value and a string as description vs. string->string in your project. (Just a guess)

#8 Updated by Alvaro Huarte over 10 years ago

Works here using your data with master and windows x86.
If you want, you can describe me the steps to cause the error? (I already tried what you indicated above), I can try to fix the error

Regards

#9 Updated by Giovanni Manghi over 10 years ago

Alvaro Huarte wrote:

Works here using your data with master and windows x86.
If you want, you can describe me the steps to cause the error? (I already tried what you indicated above), I can try to fix the error

Regards

Hi Alvaro, thanks for looking into this issue.

I just updated my qgis master and here it is still broken.

If you open the attached project and try add a new polygon (or edit the "landuse" column of an existing one), when you selects the value of the "landuse" attribute you can choose between "cork oak" or "holm oak", then the widget should write into the table "quercus suber" or "quercus rotundifolia", but instead it writes "cork oak" or "holm oak".

#10 Updated by Giovanni Manghi over 10 years ago

  • Affected QGIS version changed from 2.0.1 to master

#11 Updated by Alvaro Huarte over 10 years ago

Giovanni Manghi wrote:

Alvaro Huarte wrote:

Works here using your data with master and windows x86.
If you want, you can describe me the steps to cause the error? (I already tried what you indicated above), I can try to fix the error

Regards

Hi Alvaro, thanks for looking into this issue.

I just updated my qgis master and here it is still broken.

If you open the attached project and try add a new polygon (or edit the "landuse" column of an existing one), when you selects the value of the "landuse" attribute you can choose between "cork oak" or "holm oak", then the widget should write into the table "quercus suber" or "quercus rotundifolia", but instead it writes "cork oak" or "holm oak".

Hi Giovanni, I already did just that. Now, I've also updated my master branch, and still saving good values​​.
I reviewed the code and it seems ok. I do not understand what may be happening.

I use Windows XP/x86 + Qt 4.7.1.
Best regards

#12 Updated by Matthias Kuhn over 10 years ago

Just a sidenote: One thing I would very much like to see is the porting of these widgets to the new API [1] This will make debugging and maintenance much easier.

[1] http://blog.vitu.ch/10142013-1847/write-your-own-qgis-form-elements

#13 Updated by Alvaro Huarte over 10 years ago

Giovanni Manghi wrote:

Alvaro Huarte wrote:

Works here using your data with master and windows x86.
If you want, you can describe me the steps to cause the error? (I already tried what you indicated above), I can try to fix the error

Regards

Hi Alvaro, thanks for looking into this issue.

I just updated my qgis master and here it is still broken.

If you open the attached project and try add a new polygon (or edit the "landuse" column of an existing one), when you selects the value of the "landuse" attribute you can choose between "cork oak" or "holm oak", then the widget should write into the table "quercus suber" or "quercus rotundifolia", but instead it writes "cork oak" or "holm oak".

When you say... ' - but instead it writes "cork oak" or "holm oak" - ', you mean the value written to the DBF table? or the values showed in the attribute table of QGIS?

I'm talking about the values written in the file, right?

I have noticed a strange behavior when I edit the attributes using the "Edit feature form" from the "Identify results form".
Just edited a value, "Identify results form" shows bad values, but if I reselect the geometry then shows ok values.

#14 Updated by Alvaro Huarte over 10 years ago

Hi all!
Now, when I change the value of "landuse" attribute in the "Identify results form" of QGIS, it rewrites the display field and also shows the new value ("quercus suber" or "quercus rotundifolia"), not its description ("cork oak" or "holm oak").

This commit fix it:
https://github.com/ahuarte47/QGIS/commit/5bd4581cc0fb9395c07d0fbc8acd83f521fc886b

If we find the error that says Giovanni, I can finally make a PR
Best Regards

#15 Updated by Giovanni Manghi over 10 years ago

I just updated my master and still see the issue: the value map is a 2 columns table, named "value" and "description". The dropdown show a list made of the entries in the "description" column but in the table (in the dbf of a shapefile) it should write the value in the "value" column.

If the edits are done directly in the table of attributes the issue is clear.

If using the "edit form" (from the identify dialog) then it seems to write thr right value, but if closing the dialog and identifying again it will show anyway the wrong value.

#16 Updated by Alvaro Huarte over 10 years ago

Giovanni Manghi wrote:

I just updated my master and still see the issue: the value map is a 2 columns table, named "value" and "description". The dropdown show a list made of the entries in the "description" column but in the table (in the dbf of a shapefile) it should write the value in the "value" column.

If the edits are done directly in the table of attributes the issue is clear.

If using the "edit form" (from the identify dialog) then it seems to write thr right value, but if closing the dialog and identifying again it will show anyway the wrong value.

Sorry, it works fine here.

I am using this single modification to avoid a bug in identify dialog (When just a new landuse value is selected in edit form, the identify dialog improperly rewrites the display field and also shows the new value not its description):
https://github.com/ahuarte47/QGIS/commit/5bd4581cc0fb9395c07d0fbc8acd83f521fc886b

Giovanni, you apply this change?

If you did it, although it makes little sense, you may send me the following files?
+ qgsidentifyresultsdialog.cpp
+ qgsattributetypedialog.cpp
+ qgsattributetypeloaddialog.cpp

Thanks for you patience :-) !
Regards

#17 Updated by Giovanni Manghi over 10 years ago

weird, is still broke here, see attached screencast.

#18 Updated by Alvaro Huarte over 10 years ago

  • Pull Request or Patch supplied changed from No to Yes
  • Assignee set to Alvaro Huarte

Hi Giovanni, thank you very much for this video.

I think this pull solve the bug
https://github.com/qgis/QGIS/pull/1037

Kind Regards

#19 Updated by Alvaro Huarte about 10 years ago

Giovanni Manghi wrote:

weird, is still broke here, see attached screencast.

Hi Giovanni, maybe I've missed something, have you tried this?
Kind Regards

Alvaro

#20 Updated by Giovanni Manghi about 10 years ago

Alvaro Huarte wrote:

Giovanni Manghi wrote:

weird, is still broke here, see attached screencast.

Hi Giovanni, maybe I've missed something, have you tried this?
Kind Regards

Alvaro

Hi Alvaro,
I didn't had the time to compile qgis and test the patch, yet.

cheers!

#21 Updated by Alvaro Huarte about 10 years ago

Giovanni Manghi wrote:

Alvaro Huarte wrote:

Giovanni Manghi wrote:

weird, is still broke here, see attached screencast.

Hi Giovanni, maybe I've missed something, have you tried this?
Kind Regards

Alvaro

Hi Alvaro,
I didn't had the time to compile qgis and test the patch, yet.

cheers!

Ok, I did not know if I'd left something to alert
Best Regards

#22 Updated by Matthias Kuhn about 10 years ago

Giovanni, any progress on this?

#23 Updated by Giovanni Manghi about 10 years ago

Matthias Kuhn wrote:

Giovanni, any progress on this?

Hi Matthias,

these days I'm not able (lack of time, sleep deprivation) to manually apply patches, but if someone will commit Alvaro's patch I will be able to immediately after compile and test.

#24 Updated by Matthias Kuhn about 10 years ago

I don't know, if this PR really fixes the described problem. If I am not mistaken, this problem is related to the valuemap widget, while the code in the PR is related only to the identify results dialog. So I'd be happy if somebody could test this fix before it's being merged. I'm very tight in time as well at the moment.

#25 Updated by Matthias Kuhn about 10 years ago

  • Status changed from Open to Feedback

#26 Updated by Giovanni Manghi about 10 years ago

Matthias Kuhn wrote:

I don't know, if this PR really fixes the described problem. If I am not mistaken, this problem is related to the valuemap widget, while the code in the PR is related only to the identify results dialog. So I'd be happy if somebody could test this fix before it's being merged. I'm very tight in time as well at the moment.

Hi Alvaro, it would be a problem (don't want to abuse of your availability) create an installer with this patch?

#27 Updated by Alvaro Huarte about 10 years ago

Giovanni Manghi wrote:

Matthias Kuhn wrote:

I don't know, if this PR really fixes the described problem. If I am not mistaken, this problem is related to the valuemap widget, while the code in the PR is related only to the identify results dialog. So I'd be happy if somebody could test this fix before it's being merged. I'm very tight in time as well at the moment.

Hi Alvaro, it would be a problem (don't want to abuse of your availability) create an installer with this patch?

Hi Giovanni, no problem, I will build the installer soon
Alvaro

#28 Updated by Alvaro Huarte about 10 years ago

Giovanni Manghi wrote:

Matthias Kuhn wrote:

I don't know, if this PR really fixes the described problem. If I am not mistaken, this problem is related to the valuemap widget, while the code in the PR is related only to the identify results dialog. So I'd be happy if somebody could test this fix before it's being merged. I'm very tight in time as well at the moment.

Hi Alvaro, it would be a problem (don't want to abuse of your availability) create an installer with this patch?

Done:
http://www.filedropper.com/qgis-osgeo4w-210-i8818-setup-x86

When QGIS starts it is possible that a python error is showed. It is not related with this patch, it come from my installer.
Best Regards

Alvaro

#29 Updated by Giovanni Manghi about 10 years ago

  • Status changed from Feedback to Open

Done:
http://www.filedropper.com/qgis-osgeo4w-210-i8818-setup-x86

Hi Alvaro, many thanks for your time!

I have tested your patched QGIS and unfortunately it is still not working.

#30 Updated by Alvaro Huarte about 10 years ago

I have tested your patched QGIS and unfortunately it is still not working.

Very weird! I follow step by step your video and It works fine here :-(

One question about another issue:
Why QGIS does not draw current selected features as identify tool does ? Then it would not need to repaint the entire map (... with a many features it has bad feedback), I have study the code and seems "easy" to change it. There is one reason that I don't see ?

Thank you very much!

#31 Updated by Giovanni Manghi about 10 years ago

Alvaro Huarte wrote:

I have tested your patched QGIS and unfortunately it is still not working.

Very weird! I follow step by step your video and It works fine here :-(

at one point after closing the table and reopening it, i have seen the correct values (the ones defined in the "value" column of the map), but then after editing the table again they disappeared and instead the "description" value is shown. Then after this event I wasn't able anymore to see the correct values. Anyway in older qgis releases the substitution between the values in "description" and "value" was "on the fly", during the edition of the table and wasn't necessary to save and/or close the table to see the right values.

One question about another issue:
Why QGIS does not draw current selected features as identify tool does ? Then it would not need to repaint the entire map (... with a many features it has bad feedback), I have study the code and seems "easy" to change it. There is one reason that I don't see ?

not sure, better ask in the dev mailing list

thanks!

#32 Updated by Giovanni Manghi about 10 years ago

  • Subject changed from "value map" widget broken in qgis 2.0.1 to "value map" widget broken

#33 Updated by Alvaro Huarte about 10 years ago

Giovanni Manghi wrote:

... I have seen the correct values (the ones defined in the "value" column of the map), but then after editing the table again they disappeared and instead the "description" value is shown.

Uf, sorry, I missed, QGIS have to display descriptions, not the values​​, right?
Alvaro

#34 Updated by Alvaro Huarte about 10 years ago

Alvaro Huarte wrote:

One question about another issue:
Why QGIS does not draw current selected features as identify tool does ? Then it would not need to repaint the entire map (... with a many features it has bad feedback), I have study the code and seems "easy" to change it. There is one reason that I don't see ?

not sure, better ask in the dev mailing list

Hi Giovanni, I asked to dev list, thanks!

#35 Updated by Jürgen Fischer about 10 years ago

  • Status changed from Open to Closed

#36 Updated by Giovanni Manghi about 10 years ago

  • Target version changed from Future Release - High Priority to Version 2.2
  • Status changed from Closed to Reopened

Jürgen Fischer wrote:

Fixed in changeset 48427e1877cdb762af14f31be85b984a490828e2.

it does not seems to work on the latest master.

#37 Updated by Alvaro Huarte about 10 years ago

Hi, I proposed a slightly different change:

https://github.com/qgis/QGIS/pull/1037

Best Regards
Alvaro

#38 Updated by Jürgen Fischer about 10 years ago

Giovanni Manghi wrote:

Jürgen Fischer wrote:

Fixed in changeset 48427e1877cdb762af14f31be85b984a490828e2.

it does not seems to work on the latest master.

hm, works fine for me. If I change select a different description in the form and save that, the description also changes in the identify results. All cork oaks now.

#39 Updated by Jürgen Fischer about 10 years ago

  • Status changed from Reopened to Feedback

#40 Updated by Giovanni Manghi about 10 years ago

  • Status changed from Feedback to Open

Jürgen Fischer wrote:

Giovanni Manghi wrote:

Jürgen Fischer wrote:

Fixed in changeset 48427e1877cdb762af14f31be85b984a490828e2.

it does not seems to work on the latest master.

hm, works fine for me. If I change select a different description in the form and save that, the description also changes in the identify results. All cork oaks now.

I'm preparing a screencast to show the issue and to show how it worked in qgis 1.8

#41 Updated by Jürgen Fischer about 10 years ago

  • Status changed from Open to Feedback

#42 Updated by Giovanni Manghi about 10 years ago

  • Status changed from Feedback to Open

Jürgen Fischer wrote:

Giovanni Manghi wrote:

Jürgen Fischer wrote:

Fixed in changeset 48427e1877cdb762af14f31be85b984a490828e2.

it does not seems to work on the latest master.

hm, works fine for me. If I change select a different description in the form and save that, the description also changes in the identify results. All cork oaks now.

attached a screencast that shows the difference between qgis 1.8 and 2.0/master:

in qgis 1.8 the user can choose the attribute using a dropdown that shows what is defined in the column description of the value map, but qgis then writes (and immediately shows) what is found in the column *value" of the value map.

in qgis 2.0/master the user choose the attribute using a dropdown that shows what is defined in the column description of the value map, but qgis then shows what is found in the same *description" of the value map.

As you can see from the screencast qgis 2.0/master actually writes in the column what is found in the column *value" of the value map, but that's not the (only) point, the right/expected behaviour is the one that the users used to see in qgis 1.8, the user choose by "description" and qgis writes/presents what is found in "value".

Please note that in both qgis 1.8/2.0(master) the identify shows the attribute as the one defined in the column description of the value map, and to me is wrong as it should show anyway what is defined in the column value of the value map.

http://www.faunalia.eu/~gmanghi/qgis_tracker/value_map.mp4

#43 Updated by Jürgen Fischer about 10 years ago

Giovanni Manghi wrote:

attached a screencast that shows the difference between qgis 1.8 and 2.0/master:

in qgis 1.8 the user can choose the attribute using a dropdown that shows what is defined in the column description of the value map, but qgis then writes (and immediately shows) what is found in the column *value" of the value map.

in qgis 2.0/master the user choose the attribute using a dropdown that shows what is defined in the column description of the value map, but qgis then shows what is found in the same *description" of the value map.

I'd consider that a bug in 1.8. The attribute table shows the descriptions of value maps when you open it (unfortunately the sample data use in the screencast initially contains values outside of the map - so that doesn't show). So I think that shouldn't change if you change a value. You should be able to select a different entry by description, get the corresponding value in the data and also see the corresponding description in the attribute table (as in all other rows). 2.0 does that, apparently 1.8 didn't. The stored data is correct in both cases, isn't it?

Please note that in both qgis 1.8/2.0(master) the identify shows the attribute as the one defined in the column description of the value map, and to me is wrong as it should show anyway what is defined in the column value of the value map.

That is the intended behaviour. The point of the value maps is to enable the user to work with descriptions without having to know the values.

#44 Updated by Giovanni Manghi about 10 years ago

Jürgen Fischer wrote:

The stored data is correct in both cases, isn't it?

yes.

I'm not sure (still thinking) that the actual behavior is better for the user, but anyway if it is by design then there is no issue.

All the confusion then arise because we (me and many others) assumed that the right behavior was the one observed in qgis 1.8.

If the design is to never show the value of the "value" column of the map then it is right.

That is the intended behaviour. The point of the value maps is to enable the user to work with descriptions without having to know the values.

what about if the user uses the column in a field calculator expression? is used the "description" or the "value"?

#45 Updated by Jürgen Fischer about 10 years ago

  • Status changed from Open to Closed

Giovanni Manghi wrote:

I'm not sure (still thinking) that the actual behavior is better for the user, but anyway if it is by design then there is no issue.

Why should a changed field show the value and all other rows the description - and if closed and reopened show the description again?

what about if the user uses the column in a field calculator expression? is used the "description" or the "value"?

The value. But a function that produces the description from the value would probably be helpful for such things.

#46 Updated by matteo ghetta over 9 years ago

  • Status changed from Closed to Reopened
  • Target version changed from Version 2.2 to Version 2.6

Hi, I have the same problem reported by Giovanni in the 2.5 version.

#47 Updated by Alvaro Huarte over 9 years ago

  • Assignee changed from Alvaro Huarte to Jürgen Fischer

matteo ghetta wrote:

Hi, I have the same problem reported by Giovanni in the 2.5 version.

Hi, see the comment (#8818-43) by Jürgen Fischer:

"The point of the value maps is to enable the user to work with descriptions without having to know the values", the old behavior can be considered a bug in 1.8.

Best regards

#48 Updated by Alvaro Huarte over 9 years ago

  • Assignee deleted (Jürgen Fischer)

#49 Updated by Matthias Kuhn over 9 years ago

  • Status changed from Reopened to Closed
  • Resolution set to wontfix

Also available in: Atom PDF