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

Replace deprecated IndexFormatter #3857

Merged
merged 4 commits into from
Sep 16, 2020

Conversation

stephenworsley
Copy link
Contributor

🚀 Pull Request

Description

Addresses #3848 (and the duplicate issue #3821).

The replacement formatter is not designed to be an exact copy of IndexFormatter since it seems that IndexFormatter can be considered bugged. Specifically, IndexFormatter would add a duplicate of the label at the 0 tick to the -1 tick. This behaviour was being accounted for by our tests, which have been changed to reflect the new behaviour. The new behaviour also relies on ticks being located precisely on integers (the IndexFormatter would perform some rudimentary rounding operations). This should always be the case since the locator is MaxNLocator(integer=True).

Copy link
Contributor

@trexfeathers trexfeathers left a comment

Choose a reason for hiding this comment

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

Thanks @stephenworsley ❤️

I'm satisfied that:

  • We already have enough test coverage to allow for the new behaviour
    (since you had to change an expected result as part of this change, for a valid reason)
  • Nothing has broken
    (since all the tests pass)

Please could you address my one comment, and I think we'll be done 🙂

lib/iris/plot.py Outdated Show resolved Hide resolved
@trexfeathers trexfeathers merged commit c49af3b into SciTools:master Sep 16, 2020
tkknight added a commit to tkknight/iris that referenced this pull request Sep 20, 2020
* master:
  Whatsnew for effects on aux factories of units defaulting to 'unknown'. (SciTools#3870)
  Whatsnew entry for SciTools#3867. (SciTools#3868)
  Developer guide overhaul (SciTools#3852)
  Update CF standard name table to v75 (SciTools#3867)
  Link to new classes and methods in the Ancillary variables whatsnew. (SciTools#3865)
  update black version (SciTools#3866)
  Fix whatsnew api links. (SciTools#3856)
  Add additional pre-commit hooks (SciTools#3862)
  update pre-commit flake8 version (SciTools#3863)
  whatsnew - update announcement (SciTools#3861)
  whatsnew - remove contents directive (SciTools#3859)
  whatsnew - links and versions (SciTools#3853)
  Replace deprecated IndexFormatter (SciTools#3857)
  whatsnew for SciTools#3681 (SciTools#3858)
  Whatsnew entry for SciTools#3846. (SciTools#3855)
  Image tests: set agg backend after rcdefaults (SciTools#3846)
  whatnew - announcements (SciTools#3850)
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