Skip to content

Remove custom polar behaviour in LogLocator #24439

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

Merged
merged 1 commit into from
Nov 17, 2022

Conversation

dstansby
Copy link
Member

@dstansby dstansby commented Nov 12, 2022

PR Summary

As part of investigating #24383, I noticed that LogLocator has custom behaviour for polar Axes. This dates back 13 years (!!) to 1fdd5a8. Since log-scaled polar Axes are broken anyway, I thought I would remove this logic and see if any existing tests break.

I've also added a new test to check the y-ticks on a log-scaled polar plot. Prior to this PR the ticks were given at [ 0.01 0.1 1. 10. ], not including the max limit of 100, showing that the custom polar logic in LogLocator was wrong anyway.

PR Checklist

Tests and Styling

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (install flake8-docstrings and run flake8 --docstring-convention=all).

Documentation

  • Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • New plotting related features are documented with examples.

Release Notes

  • New features are marked with a .. versionadded:: directive in the docstring and documented in doc/users/next_whats_new/
  • API changes are marked with a .. versionchanged:: directive in the docstring and documented in doc/api/next_api_changes/
  • Release notes conform with instructions in next_whats_new/README.rst or next_api_changes/README.rst

@dstansby
Copy link
Member Author

Looks like it's fine to remove this - not sure whether best to do it here or as part of a larger PR that (hopefully, I haven't done any more yet!) fixes polar log scales?

@dstansby dstansby marked this pull request as ready for review November 13, 2022 10:15
@tacaswell tacaswell added this to the v3.7.0 milestone Nov 16, 2022
@tacaswell tacaswell closed this Nov 16, 2022
@tacaswell tacaswell reopened this Nov 16, 2022
@tacaswell
Copy link
Member

power cycled to try and pick up the appveyor fixes.

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

Successfully merging this pull request may close these issues.

5 participants