NotificationsYou must be signed in to change notification settings
Fork8.1k
Star22.1k

polar plots do not properly handle units#4905

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

Jump to bottom

Closed

jrevans wants to merge3 commits intomatplotlib:masterfromjrevans:issue04

Closed

polar plots do not properly handle units#4905

jrevans wants to merge3 commits intomatplotlib:masterfromjrevans:issue04

Conversation

Copy link

jrevans commentedAug 11, 2015

PolarAxes was not properly handling unitized data

This addresses an issue in#4897.

JRE - PolarAxes was not properly handling unitzed data. This fixes it.

9338d36

tacaswell added the status: needs review label

Aug 11, 2015

tacaswell reviewed

Aug 11, 2015

View reviewed changes

lib/matplotlib/projections/polar.py

Copy link

Member

Choose a reason for hiding this comment

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

Silly question, where is this documented/implemented?

tacaswell added this to thenext point release milestone

Aug 11, 2015

QuLogic reviewed

Aug 11, 2015

View reviewed changes

lib/matplotlib/projections/polar.py

Copy link

Member

Choose a reason for hiding this comment

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

docstring

Copy link

Member

QuLogic commentedAug 12, 2015

Side note: there's no reason to prefix your commit messages with your initials; that's why you set your author info before committing anything.

QuLogic reviewed

Aug 12, 2015

View reviewed changes

lib/matplotlib/projections/polar.py Outdated

Copy link

Member

Choose a reason for hiding this comment

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

Andx here too.

WeatherGod added the status: needs revision label

Aug 12, 2015

Copy link

Member

tacaswell commentedAug 14, 2015

Can you modify or extendhttps://github.com/matplotlib/matplotlib/blob/master/lib/matplotlib/tests/test_axes.py#L384 to catch this issue?

Fixed accidental transpose of axes when dealing with units.

676308a

Added more test cases to test polar functions.

Copy link

Author

jrevans commentedAug 17, 2015

When moving my fixes over to git, I had accidentally transposed which axis was used for the unit conversion. I have fixed that.

I have also added some more test cases to test for this. I am not sure what the process is for generating the baseline images or even running the test harness anymore. It has changed dramatically from when I first wrote it up so many years ago. What is the process for this now?

same as last commit, git just didn't get everything.

6e383f5

Copy link

Member

tacaswell commentedAug 17, 2015

See
http://matplotlib.org/devel/testing.html#writing-an-image-comparison-test
for the current directions

On Mon, Aug 17, 2015, 3:54 PM James Evansnotifications@github.com wrote:

When moving my fixes over to git, I had accidentally transposed which axis
was used for the unit conversion. I have fixed that.
I have also added some more test cases to test for this. I am not sure
what the process is for generating the baseline images or even running the
test harness anymore. It has changed dramatically from when I first wrote
it up so many years ago. What is the process for this now?
—
Reply to this email directly or view it on GitHub
#4905 (comment)
.

Copy link

Member

WeatherGod commentedAug 17, 2015

Essentially, just run the test directly (after installing the latest
changess). So, from the root source directory:python lib/matplotlib/tests/test_foobar.py). It'll run and error on the new test,
but it will produce artifacts like
"result_images/test_foobar/this_new_test_image.png". Verify the new
artifacts are right and then copy them over to
"lib/matplotlib/tests/baseline_images/test_foobar/". Rerun the test to make
sure all is well, and then add and commit those images.

On Mon, Aug 17, 2015 at 3:54 PM, James Evansnotifications@github.com
wrote:

When moving my fixes over to git, I had accidentally transposed which axis
was used for the unit conversion. I have fixed that.
I have also added some more test cases to test for this. I am not sure
what the process is for generating the baseline images or even running the
test harness anymore. It has changed dramatically from when I first wrote
it up so many years ago. What is the process for this now?
—
Reply to this email directly or view it on GitHub
#4905 (comment)
.

tacaswell removed the status: needs review label

Aug 22, 2015

Copy link

Member

tacaswell commentedSep 13, 2015

ping@jrevans Any update on this?

mdboom reviewed

Oct 6, 2015

View reviewed changes

lib/matplotlib/projections/polar.py

Copy link

Member

mdboomOct 6, 2015

Choose a reason for hiding this comment

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

This shouldn't be necessary and probably indicates a bug somewhere else. Returning unicode from this function is actually what wewant. Can you describe which backend/platform you were using when you can across this problem?

mdboom self-assigned this

Oct 8, 2015

Copy link

Member

mdboom commentedOct 20, 2015

@jrevans: Any progress on addressing the questions above? We may have to punt on this for 1.5.0.

jrevans mentioned this pull request

Oct 28, 2015

outstanding issues from jpl#4897

Closed

17 tasks

tacaswell modified the milestones:next point release (1.5.0),next bug fix release (2.0.1)

Oct 29, 2015

tacaswell reviewed

Dec 28, 2015

View reviewed changes

lib/matplotlib/tests/test_axes.py

Copy link

Member

tacaswellDec 28, 2015

Choose a reason for hiding this comment

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

This needs a@cleanup decorator so global state does not leak.

QuLogic mentioned this pull request

Mar 11, 2016

polar plot incorrectly handles units of degrees/radians#6144

Open

dstansby added the topic: polar label

Jan 29, 2017

QuLogic modified the milestones:2.0.1 (next bug fix release),2.0.2 (next bug fix release)

May 3, 2017

tacaswell modified the milestones:2.1.1 (next bug fix release),2.2 (next feature release)

Oct 9, 2017

jklymak added topic: units and array ducktypes status: needs rebase labels

Oct 6, 2018

Copy link

Member

jklymak commentedOct 6, 2018

I'm not going to close this one because it looks like it was close, but is there any chance it will get some movement? Someone who loves units will need to lead the charge...