Skip to content

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

Merged
pllim merged 1 commit into
astropy:masterfrom
dhomeier:fix-bbox-limits
Oct 28, 2020
Merged

pllim merged 1 commit into
astropy:masterfrom
dhomeier:fix-bbox-limits

Conversation

@dhomeier

@dhomeier dhomeier commented Oct 27, 2020 •

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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
Copy Markdown
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

pllim commented Oct 27, 2020

Copy link
Copy Markdown
Member

@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
Copy Markdown
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

pllim commented Oct 27, 2020

Copy link
Copy Markdown
Member

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

pllim commented Oct 27, 2020

Copy link
Copy Markdown
Member

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
Copy Markdown
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
Copy Markdown
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

pllim commented Oct 27, 2020

Copy link
Copy Markdown
Member

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

@bsipocz

bsipocz commented Oct 27, 2020

Copy link
Copy Markdown
Member

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

@dhomeier

Copy link
Copy Markdown
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
Copy Markdown
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

bsipocz commented Oct 27, 2020

Copy link
Copy Markdown
Member

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
Copy Markdown
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

pllim commented Oct 28, 2020

Copy link
Copy Markdown
Member

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.

4 participants