FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

polar plots do not properly handle units by jrevans · Pull Request #4905 · matplotlib/matplotlib · GitHub

  1. matplotlib
  2. matplotlib

Repository navigation

polar plots do not properly handle units - #4905

Closed
jrevans wants to merge 3 commits into
matplotlib:masterfrom
jrevans:issue04
ConversationCommits (3)ChecksFiles changed
Closed

polar plots do not properly handle units#4905
jrevans wants to merge 3 commits into
matplotlib:masterfrom
jrevans:issue04

Conversation

jrevans commented Aug 11, 2015

Copy link
Copy Markdown

PolarAxes was not properly handling unitized data

This addresses an issue in #4897.

tacaswell reviewed Aug 11, 2015
View reviewed changes
Comment thread lib/matplotlib/projections/polar.py

tacaswell Aug 11, 2015

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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
Comment thread lib/matplotlib/projections/polar.py

QuLogic Aug 11, 2015

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

docstring

QuLogic commented Aug 12, 2015

Copy link
Copy Markdown
Member

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
Comment thread lib/matplotlib/projections/polar.py Outdated

QuLogic Aug 12, 2015

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

And x here too.

tacaswell commented Aug 14, 2015

Copy link
Copy Markdown
Member

Can you modify or extend https://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.

jrevans commented Aug 17, 2015

Copy link
Copy Markdown
Author

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 commented Aug 17, 2015

Copy link
Copy Markdown
Member

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 commented Aug 17, 2015

Copy link
Copy Markdown
Member

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 commented Sep 13, 2015

Copy link
Copy Markdown
Member

ping @jrevans Any update on this?

mdboom reviewed Oct 6, 2015
View reviewed changes
Comment thread lib/matplotlib/projections/polar.py

mdboom Oct 6, 2015

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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 self-assigned this Oct 8, 2015

mdboom commented Oct 20, 2015

Copy link
Copy Markdown
Member

@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
Closed
14 of 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
Comment thread lib/matplotlib/tests/test_axes.py

tacaswell Dec 28, 2015

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

QuLogic mentioned this pull request Mar 11, 2016
Open
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 commented Oct 6, 2018

Copy link
Copy Markdown
Member

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 commented Oct 15, 2019

Copy link
Copy Markdown
Member

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

jklymak closed this Oct 15, 2019
story645 removed this from the future releases milestone Oct 6, 2022
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

mdboom

Labels

status: needs rebase status: needs revision topic: polar topic: units and array ducktypes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

Back | FazBrowse Home | New Git URL