Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"value map" widget broken #17511

Closed
qgib opened this issue Oct 10, 2013 · 49 comments
Closed

"value map" widget broken #17511

qgib opened this issue Oct 10, 2013 · 49 comments
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter! Vectors Related to general vector layer handling (not specific data formats)
Milestone

Comments

@qgib
Copy link
Contributor

qgib commented Oct 10, 2013

Author Name: Giovanni Manghi (@gioman)
Original Redmine Issue: 8818
Affected QGIS version: master
Redmine category:vectors


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

Is a regression since 1.8.


@qgib
Copy link
Contributor Author

qgib commented Oct 10, 2013

Author Name: Matthias Kuhn (@m-kuhn)


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

@qgib
Copy link
Contributor Author

qgib commented Oct 10, 2013

Author Name: Giovanni Manghi (@gioman)


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

@qgib
Copy link
Contributor Author

qgib commented Oct 10, 2013

Author Name: Matthias Kuhn (@m-kuhn)


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?

@qgib
Copy link
Contributor Author

qgib commented Oct 10, 2013

Author Name: Giovanni Manghi (@gioman)


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.

@qgib
Copy link
Contributor Author

qgib commented Oct 10, 2013

Author Name: Matthias Kuhn (@m-kuhn)


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

@qgib
Copy link
Contributor Author

qgib commented Oct 10, 2013

Author Name: Giovanni Manghi (@gioman)


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.


  • 6338 was configured as valuemap.zip

@qgib
Copy link
Contributor Author

qgib commented Oct 11, 2013

Author Name: Matthias Kuhn (@m-kuhn)


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)

@qgib
Copy link
Contributor Author

qgib commented Dec 14, 2013

Author Name: Alvaro Huarte (@ahuarte47)


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

@qgib
Copy link
Contributor Author

qgib commented Dec 14, 2013

Author Name: Giovanni Manghi (@gioman)


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".

@qgib
Copy link
Contributor Author

qgib commented Dec 14, 2013

Author Name: Giovanni Manghi (@gioman)


  • version was changed from 2.0.1 to master

@qgib
Copy link
Contributor Author

qgib commented Dec 15, 2013

Author Name: Alvaro Huarte (@ahuarte47)


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

@qgib
Copy link
Contributor Author

qgib commented Dec 15, 2013

Author Name: Matthias Kuhn (@m-kuhn)


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

@qgib
Copy link
Contributor Author

qgib commented Dec 16, 2013

Author Name: Alvaro Huarte (@ahuarte47)


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.

@qgib
Copy link
Contributor Author

qgib commented Dec 16, 2013

Author Name: Alvaro Huarte (@ahuarte47)


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:
ahuarte47@5bd4581

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

@qgib
Copy link
Contributor Author

qgib commented Dec 17, 2013

Author Name: Giovanni Manghi (@gioman)


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.

@qgib
Copy link
Contributor Author

qgib commented Dec 17, 2013

Author Name: Alvaro Huarte (@ahuarte47)


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):
ahuarte47@5bd4581

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

@qgib
Copy link
Contributor Author

qgib commented Dec 18, 2013

Author Name: Giovanni Manghi (@gioman)


weird, is still broke here, see attached screencast.


  • 6586 was configured as valuemap.mp4

@qgib
Copy link
Contributor Author

qgib commented Dec 18, 2013

Author Name: Alvaro Huarte (@ahuarte47)


Hi Giovanni, thank you very much for this video.

I think this pull solve the bug
#1037

Kind Regards


  • pull_request_patch_supplied was changed from 0 to 1
  • assigned_to_id was configured as Alvaro Huarte

@qgib
Copy link
Contributor Author

qgib commented Jan 10, 2014

Author Name: Alvaro Huarte (@ahuarte47)


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

@qgib
Copy link
Contributor Author

qgib commented Jan 10, 2014

Author Name: Giovanni Manghi (@gioman)


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!

@qgib
Copy link
Contributor Author

qgib commented Jan 10, 2014

Author Name: Alvaro Huarte (@ahuarte47)


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

@qgib
Copy link
Contributor Author

qgib commented Jan 20, 2014

Author Name: Matthias Kuhn (@m-kuhn)


Giovanni, any progress on this?

@qgib
Copy link
Contributor Author

qgib commented Jan 20, 2014

Author Name: Giovanni Manghi (@gioman)


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.

@qgib
Copy link
Contributor Author

qgib commented Jan 24, 2014

Author Name: Matthias Kuhn (@m-kuhn)


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.

@qgib
Copy link
Contributor Author

qgib commented Jan 24, 2014

Author Name: Matthias Kuhn (@m-kuhn)


  • status_id was changed from Open to Feedback

@qgib
Copy link
Contributor Author

qgib commented Jan 24, 2014

Author Name: Giovanni Manghi (@gioman)


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?

@qgib
Copy link
Contributor Author

qgib commented Jan 24, 2014

Author Name: Alvaro Huarte (@ahuarte47)


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

@qgib
Copy link
Contributor Author

qgib commented Jan 24, 2014

Author Name: Alvaro Huarte (@ahuarte47)


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

@qgib
Copy link
Contributor Author

qgib commented Jan 25, 2014

Author Name: Giovanni Manghi (@gioman)


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.


  • status_id was changed from Feedback to Open

@qgib
Copy link
Contributor Author

qgib commented Jan 25, 2014

Author Name: Alvaro Huarte (@ahuarte47)


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!

@qgib
Copy link
Contributor Author

qgib commented Jan 26, 2014

Author Name: Giovanni Manghi (@gioman)


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!

@qgib
Copy link
Contributor Author

qgib commented Jan 26, 2014

Author Name: Giovanni Manghi (@gioman)


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

@qgib
Copy link
Contributor Author

qgib commented Jan 26, 2014

Author Name: Alvaro Huarte (@ahuarte47)


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

@qgib
Copy link
Contributor Author

qgib commented Jan 26, 2014

Author Name: Alvaro Huarte (@ahuarte47)


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!

@qgib
Copy link
Contributor Author

qgib commented Feb 8, 2014

Author Name: Jürgen Fischer (@jef-n)


Fixed in changeset "48427e1877cdb762af14f31be85b984a490828e2".


  • status_id was changed from Open to Closed

@qgib
Copy link
Contributor Author

qgib commented Feb 14, 2014

Author Name: Giovanni Manghi (@gioman)


Jürgen Fischer wrote:

Fixed in changeset "48427e1877cdb762af14f31be85b984a490828e2".

it does not seems to work on the latest master.


  • fixed_version_id was changed from Future Release - High Priority to Version 2.2
  • status_id was changed from Closed to Reopened

@qgib
Copy link
Contributor Author

qgib commented Feb 14, 2014

Author Name: Alvaro Huarte (@ahuarte47)


Hi, I proposed a slightly different change:

#1037

Best Regards
Alvaro

@qgib
Copy link
Contributor Author

qgib commented Feb 14, 2014

Author Name: Jürgen Fischer (@jef-n)


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.

@qgib
Copy link
Contributor Author

qgib commented Feb 15, 2014

Author Name: Jürgen Fischer (@jef-n)


  • status_id was changed from Reopened to Feedback

@qgib
Copy link
Contributor Author

qgib commented Feb 15, 2014

Author Name: Giovanni Manghi (@gioman)


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


  • status_id was changed from Feedback to Open

@qgib
Copy link
Contributor Author

qgib commented Feb 16, 2014

Author Name: Jürgen Fischer (@jef-n)


  • status_id was changed from Open to Feedback

@qgib
Copy link
Contributor Author

qgib commented Feb 16, 2014

Author Name: Giovanni Manghi (@gioman)


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


  • status_id was changed from Feedback to Open

@qgib
Copy link
Contributor Author

qgib commented Feb 16, 2014

Author Name: Jürgen Fischer (@jef-n)


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.

@qgib
Copy link
Contributor Author

qgib commented Feb 16, 2014

Author Name: Giovanni Manghi (@gioman)


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"?

@qgib
Copy link
Contributor Author

qgib commented Feb 16, 2014

Author Name: Jürgen Fischer (@jef-n)


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.


  • status_id was changed from Open to Closed

@qgib
Copy link
Contributor Author

qgib commented Jul 29, 2014

Author Name: matteo ghetta (@ghtmtt)


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


  • status_id was changed from Closed to Reopened
  • fixed_version_id was changed from Version 2.2 to Version 2.6

@qgib
Copy link
Contributor Author

qgib commented Jul 29, 2014

Author Name: Alvaro Huarte (@ahuarte47)


matteo ghetta wrote:

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

Hi, see the comment (#17511 (comment)) 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


  • assigned_to_id was changed from Alvaro Huarte to Jürgen Fischer

@qgib
Copy link
Contributor Author

qgib commented Jul 29, 2014

Author Name: Alvaro Huarte (@ahuarte47)


  • assigned_to_id removed Jürgen Fischer

@qgib
Copy link
Contributor Author

qgib commented Jul 30, 2014

Author Name: Matthias Kuhn (@m-kuhn)


  • status_id was changed from Reopened to Closed
  • resolution was changed from to wontfix

@qgib qgib added Bug Either a bug report, or a bug fix. Let's hope for the latter! Vectors Related to general vector layer handling (not specific data formats) labels May 24, 2019
@qgib qgib added this to the Version 2.6 milestone May 24, 2019
@qgib qgib closed this as completed May 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter! Vectors Related to general vector layer handling (not specific data formats)
Projects
None yet
Development

No branches or pull requests

1 participant