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

Enable situate_axes in matplotlib for consistency #1978

Merged
merged 1 commit into from
Feb 19, 2018
Merged

Conversation

philippjfr
Copy link
Member

Very early on when implementing holoviews we added a situate_axes option which disables axis normalization for Raster and Image types in matplotlib. This pops up as a user question fairly frequently since it is inconsistent behavior.

Fixes: #692

@jlstevens
Copy link
Contributor

Seems fine though maybe we should have a switch in hv.config for backwards compatibility? The failing tests indicate that changing this will affect the behavior of people's existing code.

@philippjfr
Copy link
Member Author

I'm okay adding that but afaik hv.config is currently undocumented which means it might as well not exist.

@jlstevens
Copy link
Contributor

The most important place to mention hv.config is in the compatibility section of the release notes, like we did for 1.8.0. And it is documented here.

@philippjfr
Copy link
Member Author

Ah okay, great. So are we just going to add new keywords to the config argument like style_18? Doesn't seem quite right, since it isn't necessarily a style.

@jlstevens
Copy link
Contributor

We can use any keywords we want in config and I imagine it would be something like config.situate_axes_18.

@philippjfr
Copy link
Member Author

We can use any keywords we want in config and I imagine it would be something like config.situate_axes_18.

That seems pretty pointless imo, how would anyone discover that keyword exists? Seems easier to just have backward compatibility notice that says you can revert to hv.plotting.mpl.RasterPlot.situate_axes=False. I'm not against a style_18 flag or similar but I don't think every single tiny change should have its own entry in hv.config.

@jlstevens
Copy link
Contributor

I'm not against a style_18 flag or similar but I don't think every single tiny change should have its own entry in hv.config.

I do agree but if there are enough changes then they should go in a hv.config flag.

@philippjfr philippjfr force-pushed the situate_axes branch 2 times, most recently from 9b64830 to a05d53d Compare November 1, 2017 03:33
@philippjfr
Copy link
Member Author

I don't think this needs its own flag and I'm not aware of any other changes this could be grouped in with, so I'd prefer just to get this merged.

@jlstevens
Copy link
Contributor

Ok. Though at the very least this will need to be mentioned wrt backwards compatibility in the new CHANGELOG.

@jlstevens jlstevens merged commit ecfb67a into master Feb 19, 2018
@philippjfr philippjfr deleted the situate_axes branch February 20, 2018 03:35
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants