figure option dialog does not properly handle units #4909
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…is fixes it so that unit information is not lost.
| axes.xaxis.converter = xconverter | ||
| axes.yaxis.converter = yconverter | ||
| axes.xaxis.set_units( xunits ) | ||
| axes.yaxis.set_units( yunits ) |
|
Travis is only complaining about the PEP8 errors I noted. The change makes sense. At some point, I should update the editor to be mplot3d-friendly... |
| @@ -101,7 +107,7 @@ def figure_edit(axes, parent=None): | ||
| has_curve = bool(curves) | ||
|
|
||
| datalist = [(general, "Axes", "")] | ||
| if has_curve: | ||
| if has_curve and curves: |
There was a problem hiding this comment.
What is the and curves doing?
|
What mechanism in that call back is re-setting the converters/units? That seems like the actually and this is just treating the symptoms. |
|
The spacing is now cleaned up as per your style rules. As for the root of the problem, when the apply callback happens it takes the values for the limits specified in the dialog (which do not have any unit information) and uses those to 'set_xlim' and 'set_ylim', which will set the units on the respective axis based on the values passed in. Since the limit values have become unitless in the GUI they stay unitless during the 'set_*' call and the axes forget what the units should be. This will preserve that unit information and make sure it stay in place after an apply is called. As for the 'and curves', If you have a plot with a line, whose label is set to 'nolegend', then the logic fails as 'curves' is an empty list. I opted for the simplest fix of making sure that 'curves' was not empty after the preprocessing step (in the event the logic of the preprocessing step changes in the future). |
tacaswell
added a commit
that referenced
this pull request
Sep 13, 2015
FIX: Qt figure options preserve units
ksunden
added a commit
to ksunden/matplotlib
that referenced
this pull request
Oct 31, 2024
The replacement is the get/set_converter method. This includes changes to use the getter and the private setter in the qt figure options dialog menu. The choice to use the private setter was a defensive one as the public setter prevents being called multiple times (though does short circuit if an identical input is provided, which I think is actually true here, therefore the public one is probably functional (and a no-op).) It is not clear to me on analysis how the unit information is or was lost. A quick test commenting out the two lines which reset converter/units displayed no obvious detrimental effect to removing those, suggesting that even if once they were necessary, they may no longer be. These lines were last touched in matplotlib#24141, though that really only generalized the code into a loop rather than copy/pasted x and y behavior. The original inclusion of resetting was in matplotlib#4909, which indicated that the dialog reset unit info. AFAICT, that is no longer true, though I have not rigorously proved that.
5 tasks
ksunden
added a commit
to ksunden/matplotlib
that referenced
this pull request
Oct 31, 2024
The replacement is the get/set_converter method. This includes changes to use the getter and the private setter in the qt figure options dialog menu. The choice to use the private setter was a defensive one as the public setter prevents being called multiple times (though does short circuit if an identical input is provided, which I think is actually true here, therefore the public one is probably functional (and a no-op).) It is not clear to me on analysis how the unit information is or was lost. A quick test commenting out the two lines which reset converter/units displayed no obvious detrimental effect to removing those, suggesting that even if once they were necessary, they may no longer be. These lines were last touched in matplotlib#24141, though that really only generalized the code into a loop rather than copy/pasted x and y behavior. The original inclusion of resetting was in matplotlib#4909, which indicated that the dialog reset unit info. AFAICT, that is no longer true, though I have not rigorously proved that.
tacaswell
pushed a commit
that referenced
this pull request
Oct 31, 2024
* Add explicit converter setting to Axis Closes #19229 The replacement is the get/set_converter method. This includes changes to use the getter and the private setter in the qt figure options dialog menu. The choice to use the private setter was a defensive one as the public setter prevents being called multiple times (though does short circuit if an identical input is provided, which I think is actually true here, therefore the public one is probably functional (and a no-op).) It is not clear to me on analysis how the unit information is or was lost. A quick test commenting out the two lines which reset converter/units displayed no obvious detrimental effect to removing those, suggesting that even if once they were necessary, they may no longer be. These lines were last touched in #24141, though that really only generalized the code into a loop rather than copy/pasted x and y behavior. The original inclusion of resetting was in #4909, which indicated that the dialog reset unit info. AFAICT, that is no longer true, though I have not rigorously proved that.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The figure option dialog resets the axes and all unit information. This saves the axes converter information so that it can be restored when the dialog values are applied to the figure.
This addresses an issue in #4897.