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

Handle invert_axes for all Element types #1919

Merged
merged 12 commits into from Oct 7, 2017

Conversation

Projects
None yet
2 participants
@philippjfr
Member

philippjfr commented Sep 24, 2017

We never implemented invert_axes for all Element types. This code does that ensuring that any plot can now be adjoined.

Fixes:

@philippjfr philippjfr added this to the v1.9 milestone Sep 24, 2017

@jlstevens

This comment has been minimized.

Member

jlstevens commented Sep 25, 2017

Looks good although I was hoping to see cruft removed with a replacement at the level of ElementPlot or some such base class.

Happy to merge once the tests pass, ideally with a few new unit tests.

@philippjfr

This comment has been minimized.

Member

philippjfr commented Sep 25, 2017

This is not ready to review or merge.

@philippjfr philippjfr added the WIP label Sep 25, 2017

@jlstevens

This comment has been minimized.

Member

jlstevens commented Sep 25, 2017

This is not ready to review or merge.

I know! Which is why I kept my comments general and checked you hadn't added the WIP tag (yet).

@philippjfr

This comment has been minimized.

Member

philippjfr commented Oct 6, 2017

Looks good although I was hoping to see cruft removed with a replacement at the level of ElementPlot or some such base class.

Not really possible, a baseclass can't know how to properly invert the data for a particular Element.

philippjfr added some commits Oct 6, 2017

@philippjfr philippjfr removed the WIP label Oct 7, 2017

@philippjfr

This comment has been minimized.

Member

philippjfr commented Oct 7, 2017

Ready to merge.

@jlstevens

This comment has been minimized.

Member

jlstevens commented Oct 7, 2017

I'll merge though I do have one question.

Not really possible, a baseclass can't know how to properly invert the data for a particular Element.

Perhaps this could be made easy at the data API level?

@jlstevens jlstevens merged commit 3b11689 into master Oct 7, 2017

4 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.05%) to 79.913%
Details
s3-reference-data-cache Test data is cached.
Details
@philippjfr

This comment has been minimized.

Member

philippjfr commented Oct 7, 2017

If it was just about invert_axes I'd definitely agree with you, but in many cases it's tied up with the code to invert_xaxis and invert_yaxis, which definitely isn't a data API level concept.

@philippjfr philippjfr deleted the mpl_invert_axes branch Oct 9, 2017

@pyup-bot pyup-bot referenced this pull request Nov 3, 2017

Closed

Update holoviews to 1.9.0 #104

@pyup-bot pyup-bot referenced this pull request Nov 13, 2017

Closed

Update holoviews to 1.9.1 #120

@pyup-bot pyup-bot referenced this pull request Dec 12, 2017

Merged

Update holoviews to 1.9.2 #139

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment