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

FIX: return points rather than path to fix regression#14451

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
tacaswell merged 1 commit intomatplotlib:masterfrombsipocz:keep_only_points
Jun 8, 2019

Conversation

bsipocz
Copy link
Contributor

Regression in an astropy plotting example (astropy/astropy#8792) is due to#11407

Locally this PR fixes the issue and the test seem to be still working, but I didn't run the whole suite locally.

Please advise what kind of test is preferred to be added for this.

pllim reacted with hooray emoji
@tacaswelltacaswell added this to thev2.2.5 milestoneJun 4, 2019
@jklymak
Copy link
Member

Can you reproduce the error in matplotlib somehow? I can’t really tell what the issue is from the astropy example.

@bsipocz
Copy link
ContributorAuthor

bsipocz commentedJun 5, 2019
edited
Loading

The problem is that we wanted to plot points rather than paths. This example from the issue in fact doesn't use astropy:

import numpy as npimport matplotlib.pyplot as pltplt.figure(figsize=(8, 4))plt.subplot(111, projection="aitoff")x = [0, np.pi/4, np.pi/2]y = [0, np.pi/4, 3*np.pi/8]plt.scatter(x, y, color='tab:orange')plt.plot(x, y,         marker='o', linestyle='none', markersize=2, alpha=0.3)

@jklymak
Copy link
Member

Great - I guess this is fine - if@anntzer had a reason for changing paths to points, I'm sure he'd have written a test to make sure we didn't re-break it on him 😛 But I actually suspect this was just a typo.

I'm not 100% clear why this requires a projection to show up. A test would be very useful - if there was a way to make it not be an image test, even better, but sometimes those are unavoidable.

@jklymakjklymak requested a review fromanntzerJune 5, 2019 01:07
Copy link
Contributor

@anntzeranntzer left a comment

Choose a reason for hiding this comment

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

I clearly typoed in#11407, sorry about that.

bsipocz reacted with hooray emoji
@dstansby
Copy link
Member

dstansby commentedJun 5, 2019
edited
Loading

Is it possible to get theLine2D objects returned byplot() and do some checks there? Otherwise I think this might need an image test (it might be possible to modify an existing custom projection image test to exercise this though).

@tacaswell
Copy link
Member

Merged to get the back ports going for 3.1.1, will open an issue to add tests.

@tacaswelltacaswell mentioned this pull requestJun 8, 2019
@tacaswell
Copy link
Member

Thank you@bsipocz !

I am very happy that there is cross-project communication / collaboration going on (which to be fair has mostly been astropy telling Matplotlib we broke you ....).

pllim reacted with laugh emoji

@bsipocz
Copy link
ContributorAuthor

@tacaswell - I'm happy about the widening communication channels, too. I suppose it's a side effect of being upstream that we mostly report about breakage, but at least we start to run into them while testing against the dev version, even though this instance it wasn't the case.

Also, I was looking into to add a test here, too, just haven't yet got to a point to identify what I could use inLine2D that is different. If I don't open a PR by the end of the weekend about it, then I will have given up on it.

timhoffm added a commit that referenced this pull requestJun 9, 2019
…451-on-v3.1.xBackport PR#14451 on branch v3.1.x (FIX: return points rather than path to fix regression)
tacaswell added a commit that referenced this pull requestOct 22, 2019
…451-on-v2.2.xBackport PR#14451 on branch v2.2.x (FIX: return points rather than path to fix regression)
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@anntzeranntzeranntzer approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v2.2.5
Development

Successfully merging this pull request may close these issues.

5 participants
@bsipocz@jklymak@dstansby@tacaswell@anntzer

[8]ページ先頭

©2009-2025 Movatter.jp