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

CI: hotfix failing answer tests #3257

Merged
merged 1 commit into from
May 13, 2021

Conversation

neutrinoceros
Copy link
Member

@neutrinoceros neutrinoceros commented May 13, 2021

PR Summary

fix #3256 (?)
For now I'm just commenting out two tests functions that I strongly suspect are the ones generating the failing answers.
If CI validates my suspicions I'll be able to start digging into it for real.

update for posterity:
since I've rebased and force pushed a different fix, there is no trace left of the tests functions I was referring to, so here they are:

  • yt/visualization/tests/test_line_plots.test_line_plot
  • yt/visualization/tests/test_line_plots.test_multi_line_plot
    eg the only tests that use the LinePlot class (that I'm aware of)

@neutrinoceros neutrinoceros added bug blocker Highest priority level labels May 13, 2021
@neutrinoceros
Copy link
Member Author

Looks like I was right, now let's bisect it down.

@neutrinoceros
Copy link
Member Author

Good news, the issue seem to lie in an annotation method, which narrows it down much better than if it was in yt.LinePlot.init

@neutrinoceros
Copy link
Member Author

neutrinoceros commented May 13, 2021

After a couple iterations I established that calling any of the following methods causes the ticks to be inverted:

  • LinePlot.annotate_legend
  • LinePlot.set_x_unit
  • LinePlot.set_unit
  • LinePlot.annotate_title

The common denominator is that all these use the @invalidate_plot decorator, which forces self._plot_valid = False.
Then I need to understand when that invalidation is going to affect the result and how, and it seems to quickly get a lot messier, as the dataflow goes back and forth between several classes and _setup_plot methods. It would be extremely valuable to narrow it down to the part where we force inward ticks, but atm I can't find it.

update: it's very likely this line
https://github.com/neutrinoceros/yt/blob/f20e3e7281dc893110373e165d147526d6f781e9/yt/visualization/base_plot_types.py#L97

@neutrinoceros
Copy link
Member Author

neutrinoceros commented May 13, 2021

I was able to confirm that the regression is indeed upstream (reported as matplotlib/matplotlib#20219)
So I will patch this by updating our requirements (forbid 3.4.2)

@neutrinoceros neutrinoceros changed the title hotfix failing answer tests (maybe) CI: hotfix failing answer tests May 13, 2021
@brittonsmith brittonsmith merged commit 4115a2b into yt-project:main May 13, 2021
@neutrinoceros neutrinoceros deleted the hotfix_3256 branch May 13, 2021 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocker Highest priority level bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Failing answer tests
2 participants