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

Add separate drawstyles options to Qt figureoptions dialog #4589

Merged
merged 1 commit into from
Jul 8, 2015
Merged

Add separate drawstyles options to Qt figureoptions dialog #4589

merged 1 commit into from
Jul 8, 2015

Conversation

mfitzp
Copy link
Member

@mfitzp mfitzp commented Jul 5, 2015

This fixes #4447 where it was reported that it is impossible to
undo a step drawstyle using the Qt figureoptions dialog. The step
draw style was applied as a linestyle (which is permitted in the
API) but undo 'default' option was not included in the list.

However adding the 'default' to the linestyles would mean enabling
step and changing the linstyle would require the dialog to be
reopened. Here instead a separate option set is created for
draw style (and Style is renamed to Line Style).

There is also a change of indentation for the line:

 has_curve = bool(curves)

which was not indented into the correct block. This
caused an error on any figures without visible lines.

This fixes #4447 where it was reported that it is impossible to
undo a step drawstyle using the Qt figureoptions dialog. The step
draw style was applied as a linestyle (which is permitted in the
API) but undo 'default' option was not included in the list.

However adding the 'default' to the linestyles would mean enabling
step and changing the linstyle would require the dialog to be
reopened. Here instead a separate option set is created for
draw style (and Style is renamed to Line Style).

There is also a change of indentation for the line:

     has_curve = bool(curves)

which was not indented into the correct block. This
caused an error on any figures without visible lines.
@tacaswell tacaswell added this to the next point release milestone Jul 6, 2015
@jenshnielsen
Copy link
Member

👍 Works as expected and looks like a clean solution to me

has_curve = bool(curves)

# make sure that there is at least one displayed curve
has_curve = bool(curves)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the reason for the indent change here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because with out this and with no lines you get

 Traceback (most recent call last):
  File "/home/tcaswell/other_source/matplotlib/lib/matplotlib/backends/backend_qt5.py", line 653, in edit_parameters
    figureoptions.figure_edit(axes, self)
  File "/home/tcaswell/other_source/matplotlib/lib/matplotlib/backends/qt_editor/figureoptions.py", line 95, in figure_edit
    has_curve = bool(curves)
UnboundLocalError: local variable 'curves' referenced before assignment

If you suspect this is an IPython bug, please report it at:
    https://github.com/ipython/ipython/issues
or send an email to the mailing list at ipython-dev@scipy.org

You can print a more detailed traceback right now with "%tb", or use "%debug"
to interactively debug it.

Extra-detailed tracebacks for bug-reporting purposes can be enabled via:
    %config Application.verbose_crash=True

@WeatherGod
Copy link
Member

By and large, this change makes sense. Drawstyles grew out of linestyles and it really should be separated now (it used to be a single concept). I am just concerned about an indent change.

tacaswell added a commit that referenced this pull request Jul 8, 2015
FIX: Add separate drawstyles options to Qt figureoptions dialog
@tacaswell tacaswell merged commit 9410ddb into matplotlib:master Jul 8, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Qt figure options widget can't undo step linestyle
4 participants