polar plots do not properly handle units #4905

jrevans · 2015-08-11T15:54:18Z

PolarAxes was not properly handling unitized data

This addresses an issue in #4897.

tacaswell · 2015-08-11T20:09:56Z

lib/matplotlib/projections/polar.py

Silly question, where is this documented/implemented?

QuLogic · 2015-08-11T23:54:52Z

lib/matplotlib/projections/polar.py

QuLogic · 2015-08-12T00:00:16Z

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 · 2015-08-12T00:01:13Z

lib/matplotlib/projections/polar.py

And x here too.

tacaswell · 2015-08-14T15:03:40Z

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

Added more test cases to test polar functions.

jrevans · 2015-08-17T19:54:47Z

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?

tacaswell · 2015-08-17T19:57:45Z

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 Evans notifications@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)
.

WeatherGod · 2015-08-17T19:59:50Z

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 Evans notifications@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 · 2015-09-13T00:49:05Z

ping @jrevans Any update on this?

mdboom · 2015-10-06T16:13:39Z

lib/matplotlib/projections/polar.py

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

mdboom · 2015-10-20T14:03:46Z

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

tacaswell · 2015-12-28T20:51:06Z

lib/matplotlib/tests/test_axes.py

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

jklymak · 2018-10-06T04:25:21Z

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...

jklymak · 2019-10-15T00:03:02Z

Closing this as not likely to be resurrected, but feel free to request a re-open.

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

tacaswell Aug 11, 2015

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 the next point release milestone Aug 11, 2015

QuLogic reviewed Aug 11, 2015
View reviewed changes

lib/matplotlib/projections/polar.py

Copy link

Member

QuLogic Aug 11, 2015

Choose a reason for hiding this comment

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

docstring

QuLogic reviewed Aug 12, 2015
View reviewed changes

lib/matplotlib/projections/polar.py Outdated

Copy link

Member

QuLogic Aug 12, 2015

Choose a reason for hiding this comment

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

And x here too.

WeatherGod added the status: needs revision label Aug 12, 2015

Fixed accidental transpose of axes when dealing with units.

676308a

Added more test cases to test polar functions.

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

6e383f5

tacaswell removed the status: needs review label Aug 22, 2015

mdboom reviewed Oct 6, 2015
View reviewed changes

mdboom self-assigned this Oct 8, 2015

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

tacaswell Dec 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

jklymak closed this Oct 15, 2019

story645 removed this from the future releases milestone Oct 6, 2022

Uh oh!

polar plots do not properly handle units #4905

polar plots do not properly handle units #4905

Uh oh!

Conversation

jrevans commented Aug 11, 2015

Uh oh!

tacaswell Aug 11, 2015

Choose a reason for hiding this comment

Uh oh!

QuLogic Aug 11, 2015

Choose a reason for hiding this comment

Uh oh!

QuLogic commented Aug 12, 2015

Uh oh!

QuLogic Aug 12, 2015

Choose a reason for hiding this comment

Uh oh!

tacaswell commented Aug 14, 2015

Uh oh!

jrevans commented Aug 17, 2015

Uh oh!

tacaswell commented Aug 17, 2015

Uh oh!

WeatherGod commented Aug 17, 2015

Uh oh!

tacaswell commented Sep 13, 2015

Uh oh!

mdboom Oct 6, 2015

Choose a reason for hiding this comment

Uh oh!

mdboom commented Oct 20, 2015

Uh oh!

tacaswell Dec 28, 2015

Choose a reason for hiding this comment

Uh oh!

jklymak commented Oct 6, 2018

Uh oh!

jklymak commented Oct 15, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants