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

Ref #1100 , tickmode set to invalid value in mpltools #1101

Merged
merged 1 commit into from
Aug 9, 2018

Conversation

nikhase
Copy link
Contributor

@nikhase nikhase commented Aug 8, 2018

  • basevalidator throws an error on the boolean

Associated Issue: #1100

* basevalidator throws an error on the boolean
Copy link
Contributor

@jonmmease jonmmease left a comment

Choose a reason for hiding this comment

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

Thanks @nikhase, a few comments/thought below

@@ -453,13 +453,13 @@ def prep_ticks(ax, index, ax_type, props):
else:
axis_dict['tick0'] = tick0
axis_dict['dtick'] = dtick
axis_dict['tickmode'] = False
axis_dict['tickmode'] = None
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be better to be explicit here and use 'auto' rather than None

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you maybe help me to understand what is going on? Are you trying to catch the axis scaling properties {“linear”, “log”, “symlog”, “logit”} from matplotlib.axes.Axes.set_xscale (other axis respectively)? There is no auto option. Maybe None is treated as auto?
It seems like despite the tickmode is set to None, it is inferred later on, for example the parsed JSON shows:

'layout': {'autosize': False,
               'height': 360,
               'hovermode': 'closest',
               'margin': {'b': 41, 'l': 61, 'pad': 0, 'r': 79, 't': 14},
               'showlegend': False,
               'width': 434,
               'xaxis':{...
                         'type': 'linear',
                         ...},
               'yaxis': {'anchor': 'x',
                         ...
                         'type': 'linear',
                         ...}
...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

btw, i chose None in the first place because of the code snippet axis_dict.pop('tickmode', None) in line 491.

elif scale == 'log':
try:
axis_dict['tick0'] = props['axes'][index]['tickvalues'][0]
axis_dict['dtick'] = props['axes'][index]['tickvalues'][1] - \
props['axes'][index]['tickvalues'][0]
axis_dict['tickmode'] = False
axis_dict['tickmode'] = None
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like this should be tick mode of 'log'. Would you mind trying this out on a simple matplotlib figure with a log axis?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a matplotlib log example:
image
With None, it looks like:
image

and brings the warnings:

plotly.graph_objs.Annotation is deprecated. 
...
Converted non-base10 x-axis log scale to 'linear'
...
Dang! That path collection is out of this world. I totally don't know what to do with it yet! Plotly can only import path collections linked to 'data' coordinates

With log, it looks like:
image
Same warning as in the None setting here.
I cannot see a difference between None and setting to log. What should it then be?
Despite that, there are obviously troubles converting non base10 logs.

@jonmmease
Copy link
Contributor

Thanks for looking into setting the axis type to values other than None. Based on the fact that the type is inferred to be linear later on, I think your solution of just changing False to None makes sense.

Thanks for the contribution!

@jonmmease jonmmease merged commit 2d264f7 into plotly:master Aug 9, 2018
@nikhase nikhase deleted the mpltools-tickmode-bugfix branch August 9, 2018 12:20
@jonmmease jonmmease added this to the v3.1.1 milestone Aug 10, 2018
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.

None yet

2 participants