Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Basic sanity testing for 2D and 3D sketches. #318

Merged
merged 7 commits into from
Aug 18, 2021

Conversation

tushar5526
Copy link
Member

To help the maintainers review new PRs easily, I have put together some p5 sketches from the documentation covering various APIs of the p5py package.

It runs all the sketches for 2 seconds headlessly and checks for errors and prevents PRs from being merged that would break the functionality of the library.

#308 broke the 3D rendering pipeline, so I have reverted the changes. Maybe @janbehrens @ziyaointl could look into it.

@ziyaointl
Copy link
Member

Thank you! This is excellent work and I'm sure all p5py contributors would appreciate this addition to the project. I'll take a closer look at the renormalization problem in the 3D pipeline today and post an update :)

@tushar5526
Copy link
Member Author

Just to mention, we are setting up p5 multiple times for each of our actions. Instead, we can save and upload it as an artefact and save some processing power. I will cover this in a different PR soon.

@ziyaointl
Copy link
Member

Just to mention, we are setting up p5 multiple times for each of our actions. Instead, we can save and upload it as an artefact and save some processing power. I will cover this in a different PR soon.

That's a good idea! Thanks for working on it!

Also, I found the reason of why the 3D pipeline was breaking - #308 did not convert the return value to an np.array. I'll open another PR that fixes the issue after this one is merged :)

…ate the program when some error occurs. commit-credits: Mark :p
@tushar5526 tushar5526 force-pushed the basic-sanity-testing branch from fde19be to 9703cad Compare August 18, 2021 19:58
@ziyaointl
Copy link
Member

Thank you @tushar5526 for your timely responses! I'll go ahead and merge this PR :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants