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

Fix/rectangle to polygon #4145

Merged
merged 14 commits into from
Mar 19, 2023
Merged

Conversation

hippo91
Copy link
Contributor

@hippo91 hippo91 commented Mar 17, 2023

Overview

This PR address the problem of having a Rectangle function that behaves as a Quadrilateral factory.

resolves #1604

Details

  • The Rectangle function has been renamed in Quadrilateral.
  • A new Rectangle function has been created.
  • It checks that only three points are given. If four points are given it raises a deprecation warning else if more or less than 3 points are given a TypeError is raised.
  • The 3 points can be given in arbitrary order but should be different and defines orthogonal vectors otherwise ValueError is raised
  • Extensive tests have been added.

@github-actions github-actions bot added the bug Uh-oh! Something isn't working as expected. label Mar 17, 2023
@codecov
Copy link

codecov bot commented Mar 17, 2023

Codecov Report

Merging #4145 (fa51c38) into main (d832619) will increase coverage by 0.00%.
The diff coverage is 97.29%.

@@           Coverage Diff           @@
##             main    #4145   +/-   ##
=======================================
  Coverage   95.56%   95.57%           
=======================================
  Files          95       95           
  Lines       20247    20282   +35     
=======================================
+ Hits        19350    19384   +34     
- Misses        897      898    +1     

Co-authored-by: Tetsuo Koyama <tkoyama010@gmail.com>
Copy link
Contributor

@MatthewFlamm MatthewFlamm left a comment

Choose a reason for hiding this comment

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

Excellent first PR! Thanks. I have not yet looked through the tests, but some comments to get started.

pyvista/utilities/geometric_objects.py Outdated Show resolved Hide resolved
pyvista/utilities/geometric_objects.py Outdated Show resolved Hide resolved
pyvista/utilities/geometric_objects.py Outdated Show resolved Hide resolved
pyvista/utilities/geometric_objects.py Show resolved Hide resolved
pyvista/utilities/geometric_objects.py Outdated Show resolved Hide resolved
pyvista/utilities/geometric_objects.py Outdated Show resolved Hide resolved
pyvista/utilities/geometric_objects.py Outdated Show resolved Hide resolved
hippo91 and others added 7 commits March 17, 2023 15:21
Copy link
Member

@akaszynski akaszynski left a comment

Choose a reason for hiding this comment

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

I made a few changes, including aligning it with our way of running tests and checking for warnings/errors.

Thanks for your work!

@hippo91
Copy link
Contributor Author

hippo91 commented Mar 18, 2023

@tkoyama010 @akaszynski @adeak @MatthewFlamm thanks for your review.
Is there something more i can do with this PR?

@akaszynski
Copy link
Member

@tkoyama010 @akaszynski @adeak @MatthewFlamm thanks for your review. Is there something more i can do with this PR?

We generally wait 24 hours to allow all contributors to say their peace before merging. Expect it merged tomorrow.

Next release is 0.39.0, probably by the end of this month or sooner.

@tkoyama010
Copy link
Member

Great first contribution. Thank you!

@akaszynski akaszynski merged commit 95d819f into pyvista:main Mar 19, 2023
assert np.allclose(mesh.points, pt_tuples)


def test_rectangle_not_orthognal_entries():
Copy link
Member

Choose a reason for hiding this comment

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

Consider adding a docstring for the test_rectangle_not_orthognal_entries function to provide more context and describe its purpose.

Suggested change
def test_rectangle_not_orthognal_entries():
def test_rectangle_not_orthogonal_entries():
"""Test the error handling when the input points are not orthogonal."""

Copy link
Member

Choose a reason for hiding this comment

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

(this is here for testing of another library)

@akaszynski akaszynski mentioned this pull request Apr 30, 2023
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Uh-oh! Something isn't working as expected.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rectangle --> Quadrilateral
5 participants