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

Image tests: set agg backend after rcdefaults #3846

Merged
merged 2 commits into from
Sep 16, 2020

Conversation

rcomer
Copy link
Member

@rcomer rcomer commented Sep 15, 2020

I was following @tkknight's nice new instructions for installing and running tests, but got a bunch of errors and failures from assertBoundsTickLabels and assertPointsTickLabels on the TestGraphicStringCoord class. The errors look like this:

Traceback (most recent call last):
  File "[git-path]/iris/lib/iris/tests/unit/plot/test_plot.py", line 57, in test_xaxis_labels_with_axes
    self.assertBoundsTickLabels("xaxis", ax)
  File "[git-path]/iris/lib/iris/tests/unit/plot/__init__.py", line 45, in assertBoundsTickLabels
    actual = self.tick_loc_and_label(axis, axes)
  File "[git-path]/iris/lib/iris/tests/unit/plot/__init__.py", line 34, in tick_loc_and_label
    axes.figure.canvas.draw)
  File "[site-packages-path]/matplotlib/backends/backend_tkagg.py", line 10, in draw
    _backend_tk.blit(self._tkphoto, self.renderer._renderer, (0, 1, 2, 3))
  File "[site-packages-path]/matplotlib/backends/_backend_tk.py", line 75, in blit
    photoimage.blank()
  File "[py3-path]/tkinter/__init__.py", line 3548, in blank
    self.tk.call(self.name, 'blank')
_tkinter.TclError: invalid command name "pyimage353"

so it seemed the test was attempting to use TkAgg, even though the intent in tests/__init__.py is clearly to use Agg. Switching the order of the backend setting and rcdefaults call fixed the problem.

I'm not clear why this hasn't come up for anyone else. I do have a matplotlibrc file, which probably most devs don't - this file was the reason the rcdefaults call was originally added (#2746).

@trexfeathers
Copy link
Contributor

This is being investigated in #3821 🙂

@trexfeathers
Copy link
Contributor

Sorry shouldn't have closed it, read it as an issue! @stephenworsley does this help fix #3821?

@rcomer
Copy link
Member Author

rcomer commented Sep 15, 2020

Thanks @trexfeathers, if I'd noticed that issue I might have got here quicker...!

@stephenworsley
Copy link
Contributor

I think this probably does help fix #3821. Thanks @rcomer!

matplotlib.rcdefaults()
matplotlib.use("agg")
Copy link
Member

@bjlittle bjlittle Sep 15, 2020

Choose a reason for hiding this comment

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

@rcomer Awesome spot 👀

Works for me, tested it locally 👍

Would you mind just adding a comment above matplotlib.use("agg") to ensure that a future developer doesn't move this statement back to being before matplotlib.rcdefaults(), thanks 😀

You could even quote this PR, just to add some connective tissue...

Copy link
Member

Choose a reason for hiding this comment

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

Otherwise LGTM 🥳

Copy link
Member

Choose a reason for hiding this comment

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

closes #3821

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @bjlittle, I've added that comment. Also a comment about why the rcdefaults is there for good measure.

@bjlittle bjlittle added this to the v3.0.0 milestone Sep 15, 2020
@bjlittle bjlittle self-assigned this Sep 15, 2020
@bjlittle
Copy link
Member

@rcomer Awesome, thanks 👍

@bjlittle bjlittle merged commit e524958 into SciTools:master Sep 16, 2020
trexfeathers added a commit to trexfeathers/iris that referenced this pull request Sep 16, 2020
rcomer pushed a commit that referenced this pull request Sep 16, 2020
@rcomer rcomer deleted the image-test-backend-after-defaults branch September 16, 2020 10:45
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants