NotificationsYou must be signed in to change notification settings
Fork7.9k
Star21.3k

addressing issue #29156, converted ps to array before slicing#29175

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

Merged

scottshambaugh merged 4 commits intomatplotlib:v3.9.xfromvicliu2001:v3.9.x

Dec 11, 2024

Merged

addressing issue #29156, converted ps to array before slicing#29175

scottshambaugh merged 4 commits intomatplotlib:v3.9.xfromvicliu2001:v3.9.x

Dec 11, 2024

Conversation

Copy link

Contributor

vicliu2001 commentedNov 23, 2024•
edited
Loading

PR summary

closes issue#29156

PR checklist

"closes 00000" is in the body of the PR description tolink the related issue
new and changed code istested
[N/A]Plotting related features are demonstrated in anexample
[N/A]New Features andAPI Changes are noted with adirective and release note
[ N/A] Documentation complies withgeneral anddocstring guidelines

converted ps to array before slicing

5a51e6a

github-actionsbot added the topic: mplot3d label

Nov 23, 2024

github-actionsbot reviewed

Nov 23, 2024

View reviewed changes

Copy link

github-actionsbot left a comment

Choose a reason for hiding this comment

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

Thank you for opening your first PR into Matplotlib!

If you have not heard from us in a week or so, please leave a new comment below and that should bring it to our attention. Most of our reviewers are volunteers and sometimes things fall through the cracks.

You can also join uson gitter for real-time discussion.

For details on testing, writing docs, and our review process, please seethe developer guide

We strive to be a welcoming and open project. Please follow ourCode of Conduct.

vicliu2001 changed the title~~converted ps to array before slicing, addressing issue #29156~~addressing issue #29156, converted ps to array before slicing

Nov 23, 2024

added test cases for the modification to art3d.py

bdf5514

vicliu2001 marked this pull request as ready for review

November 23, 2024 19:58

scottshambaugh reviewed

Nov 23, 2024

View reviewed changes

lib/mpl_toolkits/mplot3d/tests/test_art3d.py Outdated


		# Following code is an example taken from
		# https://stackoverflow.com/questions/18897786/transparency-for-poly3dcollection-plot-in-matplotlib
		# and modified to test _generate_normals function

Copy link

Contributor

scottshambaughNov 23, 2024•
edited
Loading

Choose a reason for hiding this comment

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

To the other devs, can we include stack overflow code directly like this? It's not clear to me if the CC BY-SA license for stack overflow posts is compatible with matplotlib's license.

Copy link

Member

oscargusDec 7, 2024

Choose a reason for hiding this comment

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

I think a better test is the one in the issue, with a note that it is a smoke test and a reference to the issue. The added test is not really testing any rendering aspects anyway.

Copy link

Member

oscargusDec 7, 2024

Choose a reason for hiding this comment

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

Better in the sense that we do not have to worry about the SE-connection and that the SE-test doesn't really add anything more, just more complicated code.

Copy link

ContributorAuthor

vicliu2001Dec 7, 2024

Choose a reason for hiding this comment

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

Hello, can you elaborate a bit more about the SE-connection and SE-test? I'm not sure what you're referring to. I have updated the test using the one in the original issue. I followed similar format as the other smoke test in the file.