Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Fixed not being able to set vertical/horizontal alignments in polar graphs#10792

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

ghost
Copy link

@ghostghost commentedMar 15, 2018
edited by ghost
Loading

PR Summary

Previously polar graphs could not have their vertical/horizontal alignments set by set_rgrids(), this fix should re-enable this functionality.

Issue #10105

PR Checklist

  • Has Pytest style unit tests
  • Code is PEP 8 compliant

pbregener reacted with thumbs up emoji
@ghost
Copy link
Author

I believethis pull request is what caused this regression.

@pbregener
Copy link

Would love to see this simple patch backported tov2.2.x as well 👍

@OD1995
Copy link

@charlesruan@pbregener is this fix live? If so, how can I take advantage of the changes?

@pbregener
Copy link

This MR is not merged, yet, so it's not "live" in the sense that it is part of any release or even current master. I guess the 5 commits should be squashed into one with a sensible commit message and then some maintainer should review and eventually merge it.

OD1995 reacted with thumbs up emoji

@ImportanceOfBeingErnest
Copy link
Member

There is also this regression on polar plots:#10882
Are they related?

@jklymakjklymak added this to thev3.0 milestoneMar 27, 2018
Copy link
Member

@phobsonphobson left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. It looks like this works as expected.

@jklymak
Copy link
Member

jklymak commentedMar 27, 2018
edited
Loading

PRs need two reviews. This looks reasonable, but it would be nice to know what PR the regression came in from and whether this was just an oversight or a design decision.

@pbregener
Copy link

@jklymak This goes back to#9068 and thereforev2.1.0. Also see the original issue for this:#10105

jklymak reacted with thumbs up emoji

@jklymak
Copy link
Member

Thanks. ping@QuLogic who is the local polar expert.

Copy link
Member

@jklymakjklymak left a comment

Choose a reason for hiding this comment

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

This looks fine to me, but reluctant to merge w/o@QuLogic having a look..

@dopplershift
Copy link
Contributor

If this really is a regression, it should probably be milestoned for v2.2.3

pbregener reacted with thumbs up emoji

@jklymak
Copy link
Member

Its a 2.1 regression. Not sure on the policy there.

@pbregener
Copy link

It is a regression and as mentioned above, I would love to see this inv2.2.x. Should be really easy to backport, too.

Not sure about how matplotlib usually handles this, but I guess it would be nice to squash the five commits for this fairly trivial change into one@charlesruan

@dopplershift
Copy link
Contributor

@tacaswell said:

I propose the following criteria for backporting PRs to the 2.2.x branch. We always will backport

  • critical bug fixes (segfault, failure to import, things that the user can not work around)
  • fixes for regressions against 2.0 or 2.1

This would seem to fit.

jklymak reacted with thumbs up emoji

@dopplershiftdopplershift modified the milestones:v3.0,v2.2.3Mar 27, 2018
@br-Zhangbr-Zhangforce-pushed thebugfix-for-issue-10105 branch from965e5d9 toad70febCompareMarch 27, 2018 21:09
@ghost
Copy link
Author

Commits have been squashed

@pbregener
Copy link

ping@jklymak and especially@QuLogic for an update on this PR?

@jklymakjklymak merged commitb33aabc intomatplotlib:masterApr 23, 2018
lumberbot-appbot pushed a commit that referenced this pull requestApr 23, 2018
@phobson
Copy link
Member

@charlesruan thanks so much for this PR and your patience with the process!

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Projects
None yet
Milestone
v2.2.3
Development

Successfully merging this pull request may close these issues.

6 participants
@pbregener@OD1995@ImportanceOfBeingErnest@jklymak@dopplershift@phobson

[8]ページ先頭

©2009-2025 Movatter.jp