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

Enable test for bbox_size limits in all Matplotlib 3 versions #10952

Merged
merged 1 commit into from Oct 28, 2020

Conversation

dhomeier
Copy link
Contributor

@dhomeier dhomeier commented Oct 27, 2020

Experimental PR to confirm or disprove the conclusions on how get_tightbbox calculation changes over Matplotlib version from #10900 (comment)
(tested locally against mpl 2.2.5, 3.0.3 and 3.3.2).

@dhomeier dhomeier marked this pull request as ready for review October 27, 2020 12:34
@dhomeier
Copy link
Contributor Author

This does "unskip" the mpl2* tests, so probably needs a changelog? And there is no provision for a Matplotlib 4.0; could add either a comment or a mark.xfail to be prepared, though I do not expect a major version bump anytime soon.

@dhomeier
Copy link
Contributor Author

Also the interface wants me to resolve the conflicts against master, while the mpl2 case is only relevant for the 4.0.x branch – not sure how to go about this.

@pllim
Copy link
Member

pllim commented Oct 27, 2020

@dhomeier , you might need to open 2 PRs for this:

  • One against master and only for matplotlib >= 3.
  • One against 4.0.x branch and for both matplotlib 2 and 3.

@dhomeier
Copy link
Contributor Author

  • One against 4.0.x branch and for both matplotlib 2 and 3.

Thanks. Since this is basically the version for 4.0.x, do you know if there is a way to change the "target" branch, or do I have to open a new PR?

@pllim
Copy link
Member

pllim commented Oct 27, 2020

Oh, weird. I used to be able to change the base branch right below the title, but I cannot seem to do that anymore. You might be able to create a new branch locally off the 4.0.x branch and then cherry pick this commit hash onto that branch, and then push that new branch out as a new PR?

@pllim pllim changed the base branch from master to v4.0.x October 27, 2020 14:17
@pllim pllim changed the base branch from v4.0.x to master October 27, 2020 14:17
@pllim
Copy link
Member

pllim commented Oct 27, 2020

Update: Okay, I found the way to change the base branch (I think they changed the UI a bit?). But unfortunately, it didn't work for your PR, as it introduced a bunch of unrelated commits. You'll have to manually do it. Thanks for your patience!

@dhomeier
Copy link
Contributor Author

OK, creating a new PR from my branch also allowed me to pick 4.0.x, but then added the entire changeset from master, so I may have to fork off a new branch from 4.0.x.

@dhomeier dhomeier modified the milestones: v4.0.4, v4.2.1 Oct 27, 2020
@dhomeier dhomeier changed the title MNT: test for different bbox_size limits in Matplotlib 2 + 3 versions Enable test for bbox_size limits in all Matplotlib 3 versions Oct 27, 2020
@dhomeier
Copy link
Contributor Author

I may have to fork off a new branch from 4.0.x.

But v4.0.x does not even have that test (last commit was 13 days ago...)

@pllim
Copy link
Member

pllim commented Oct 27, 2020

Err... right... it isn't backported yet... @bsipocz , any ideas?

@bsipocz
Copy link
Member

bsipocz commented Oct 27, 2020

Hmm, ok, I'll do a set of backports later today

@dhomeier
Copy link
Contributor Author

v4.1.x does not include it either, but since rebasing on master here the MATPLOTLIB_LT_2x markers are also gone...

@dhomeier
Copy link
Contributor Author

@bsipocz I may have become a bit confused over the milestones for this one (which is still a follow-up on #10900) – will there be v4.0.4, v4.1.1 and v4.2.1 branches in parallel now?

@bsipocz
Copy link
Member

bsipocz commented Oct 27, 2020

v4.1.x is not my business, but I don't think there will any more releases there

So, there will be a 4.0.x along with 4.2, maybe another one sooner during the RC period depending on how many fixes land in there. We already have 4.2.1 milestone as a storage for fixes that are not release blockers for 4.2. If anything there get merged before the 4.2 is out, I'll backport them and remilestone to 4.2 (this is basically the same process as it was in the past many years)

@dhomeier
Copy link
Contributor Author

Marked no-changelog, since for this branch it just restores pre-#10900 behaviour of a test that does not have a changelog entry either.

@pllim pllim merged commit 2183c11 into astropy:master Oct 28, 2020
@pllim
Copy link
Member

pllim commented Oct 28, 2020

Thanks!

bsipocz pushed a commit that referenced this pull request Nov 6, 2020
Enable test for bbox_size limits in all Matplotlib 3 versions
@bsipocz bsipocz modified the milestones: v4.2.1, v4.2 Nov 6, 2020
bsipocz pushed a commit that referenced this pull request Nov 7, 2020
Enable test for bbox_size limits in all Matplotlib 3 versions
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.

None yet

4 participants