addressing issue #29156, converted ps to array before slicing #29175
Conversation
There was a problem hiding this comment.
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 us on gitter for real-time discussion.
For details on testing, writing docs, and our review process, please see the developer guide
We strive to be a welcoming and open project. Please follow our Code of Conduct.
|
|
||
| # 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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
|
Whoops, this should have gone into the |
|
Thank you @vicliu2001 for the PR, and congratulations on your first contribution to matplotlib! We hope to see more of you! |
PR summary
closes issue #29156
PR checklist