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

Added code information to Poly3DCollection by cwebster2 · Pull Request #4793 · matplotlib/matplotlib · GitHub

  1. matplotlib
  2. matplotlib

Repository navigation

Added code information to Poly3DCollection - #4793

Merged
WeatherGod merged 1 commit into
matplotlib:masterfrom
cwebster2:issue4784
Jul 27, 2015
ConversationCommits (1)ChecksFiles changed
Merged

Added code information to Poly3DCollection#4793
WeatherGod merged 1 commit into
matplotlib:masterfrom
cwebster2:issue4784

Conversation

cwebster2 commented Jul 26, 2015

Copy link
Copy Markdown
Contributor

Fixes #4784 by adding path code information to the Poly3DCollection.
This adds two new methods, path_to_3d_segment_with_codes and
paths_to_3d_segments_with_codes which are meant to replace the
versions without "_with_codes". A method was added to PolyCollection
to allow Poly3DCollection to set vertices with path codes.

This code was adapted from a PR by @ianthomas23.

tacaswell reviewed Jul 26, 2015
View reviewed changes
Comment thread lib/matplotlib/collections.py Outdated

tacaswell Jul 26, 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

pep8 whitespace

tacaswell commented Jul 26, 2015

Copy link
Copy Markdown
Member

attn @WeatherGod

WeatherGod reviewed Jul 27, 2015
View reviewed changes
Comment thread lib/matplotlib/collections.py Outdated

WeatherGod Jul 27, 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

should also get rid of the unneeded backslash and the extraneous space at the end of the exception message.

WeatherGod commented Jul 27, 2015

Copy link
Copy Markdown
Member

by and large, this looks good to me. Should add at least an image test, probably also a 'what's new' entry to describe the new method added to PolyCollection to allow for describing collections of polygons with holes (assuming I am understanding the limitation of the old code correctly?).

cwebster2 commented Jul 27, 2015

Copy link
Copy Markdown
Contributor Author

@WeatherGod thanks. I pushed a new commit addressing the pep8 issues. I'll add the docstring, the what's new and look into the image test. You have the gist of it, polygons with holes in certain scenarios render improperly as every line ends up being a "draw" when some need to be "moves" (as I understand it). Making use of the path codes has fixed 100% of my problem cases, which was almost all of my hundreds of plots.

WeatherGod reviewed Jul 27, 2015
View reviewed changes
Comment thread lib/matplotlib/collections.py Outdated

WeatherGod Jul 27, 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

still missing the self.stale = True.

cwebster2 Jul 27, 2015

Copy link
Copy Markdown
Contributor Author

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

I caught that after I pushed the last commit. I have it in my local branch and I'll push it once I have the testcase figured out (trying to find a simplest case than my 40x40 data that reproduces the error)

Add path code information to Poly3DCollection
eb00c0c 
Fixes matplotlib#4784 by adding path code information to the Poly3DCollection.
This adds two new methods, path_to_3d_segment_with_codes and
paths_to_3d_segments_with_codes which are meant to replace the
versions without "_with_codes".  A method was added to PolyCollection
to allow Poly3DCollection to set vertices with path codes.

Add image test for a case that causes improper polygon rendering
without this fix.

This code was adapted from a PR by @ianthomas23.

cwebster2 commented Jul 27, 2015

Copy link
Copy Markdown
Contributor Author

@WeatherGod I pushed a new commit with the additions and changes mentioned above. It failed the python 3.3 CI check, but as far as I can tell that error wasn't related to my changes.

mdboom reviewed Jul 27, 2015
View reviewed changes
Comment thread lib/mpl_toolkits/mplot3d/art3d.py

mdboom Jul 27, 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

If you use izip_longest here, you don't need to allocate a large array of ones in zs first.

mdboom commented Jul 27, 2015

Copy link
Copy Markdown
Member

👍

WeatherGod commented Jul 27, 2015

Copy link
Copy Markdown
Member

The failure is a transient one that has been occurring on Travis for a while now. I restarted the test to make everything green.

WeatherGod added a commit that referenced this pull request Jul 27, 2015
Merge pull request #4793 from cwebster2/issue4784
561ce23 
Added code information to Poly3DCollection
WeatherGod merged commit 561ce23 into matplotlib:master Jul 27, 2015
cwebster2 deleted the issue4784 branch July 27, 2015 20:45
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

No one assigned

Labels

topic: mplot3d

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

Back | FazBrowse Home | New Git URL