-
Notifications
You must be signed in to change notification settings - Fork 500
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
Spherical bb #2620
Spherical bb #2620
Conversation
@shimwell Hi John here's 2620. Yes it failed again. There was one line in which I wrote bb = mesh.bounding_box() Instead of.... bb = mesh.bounding_box Obviously bounding_box is not callable. I fixed the code on my local repo, committed it to my branch spherical_bb and pushed to origin. I don't see an option to either open a new pull request on the same branch nor rerun the same CI test on the updated pull request. Should I .... make a new branch on my local repo (e.g. spherical_bb_2) and open a PR on that? There must be some way of re-running the CI tests... -Chris |
@shimwell Nevermind. i think i figured out how to re-issue the PR. I'm working on getting Docker and everything installed on my Ubuntu-partitioned MacBook I so can run these tests locally and avoid these problems in the future |
…into spherical_bb s
Looks like the tests are passing but if you still want to run the test locally here is a command pytest tests/unit_tests/test_mesh.py |
@shimwell Yes I did eventually figure that out. At first I was just navigating to /tests/ and running pytest. Since I don't have...docker and any number of necessary packages and libraries and virtual settings enabled it generate a lot of errors. I've submitted another pull request for the cylindrical mesh as well. |
tests/unit_tests/test_mesh.py
Outdated
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR @caderache2014. A few comments below:
Co-authored-by: Jonathan Shimwell <[email protected]>
@shimwell @paulromano cool |
Co-authored-by: Jonathan Shimwell <[email protected]> Co-authored-by: Paul Romano <[email protected]>
Description
This is a second attempt at adding the bounding box property to spherical meshes. The first (based on #2605) failed CI tests because the bounding box property itself was not added to the SphericalMesh class.
Fixes # (issue) #2605
Checklist