Skip to content

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 🙂

@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.

2 participants