Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
figure option dialog does not properly handle units#4909
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
…is fixesit 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 ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
PEP8: unnecessary spaces
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
What is theand 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). |
FIX: Qt figure options preserve units
The replacement is the get/set_converter method.This includes changes to use the getter and the private setter in the qtfigure options dialog menu.The choice to use the private setter was a defensive one as the publicsetter prevents being called multiple times (though does short circuitif 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 waslost. A quick test commenting out the two lines which resetconverter/units displayed no obvious detrimental effect to removingthose, suggesting that even if once they were necessary, they may nolonger be.These lines were last touched inmatplotlib#24141, though that really only generalizedthe code into a loop rather than copy/pasted x and y behavior.The original inclusion of resetting was inmatplotlib#4909, which indicated thatthe dialog reset unit info. AFAICT, that is no longer true, though Ihave not rigorously proved that.
The replacement is the get/set_converter method.This includes changes to use the getter and the private setter in the qtfigure options dialog menu.The choice to use the private setter was a defensive one as the publicsetter prevents being called multiple times (though does short circuitif 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 waslost. A quick test commenting out the two lines which resetconverter/units displayed no obvious detrimental effect to removingthose, suggesting that even if once they were necessary, they may nolonger be.These lines were last touched inmatplotlib#24141, though that really only generalizedthe code into a loop rather than copy/pasted x and y behavior.The original inclusion of resetting was inmatplotlib#4909, which indicated thatthe dialog reset unit info. AFAICT, that is no longer true, though Ihave not rigorously proved that.
* Add explicit converter setting to AxisCloses#19229The replacement is the get/set_converter method.This includes changes to use the getter and the private setter in the qtfigure options dialog menu.The choice to use the private setter was a defensive one as the publicsetter prevents being called multiple times (though does short circuitif 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 waslost. A quick test commenting out the two lines which resetconverter/units displayed no obvious detrimental effect to removingthose, suggesting that even if once they were necessary, they may nolonger be.These lines were last touched in#24141, though that really only generalizedthe code into a loop rather than copy/pasted x and y behavior.The original inclusion of resetting was in#4909, which indicated thatthe dialog reset unit info. AFAICT, that is no longer true, though Ihave not rigorously proved that.
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.