Skip to content

Comments

refactor: use point.buffer to create circular polygon#314

Open
andrefmello91 wants to merge 5 commits intofib-international:devfrom
andrefmello91:circular-section-update
Open

refactor: use point.buffer to create circular polygon#314
andrefmello91 wants to merge 5 commits intofib-international:devfrom
andrefmello91:circular-section-update

Conversation

@andrefmello91
Copy link

Use shapely default way for creating a circle.

@mortenengen mortenengen added the enhancement New feature or request label Feb 17, 2026
@mortenengen mortenengen moved this to Changes requested 📝 in PR tracker Feb 17, 2026
@mortenengen
Copy link
Member

Thanks for this improvement @andrefmello91! I have left one comment for you to consider while finalizing 😃

@andrefmello91
Copy link
Author

Thanks for this @mortenengen!
I brought back npoints with the default value of 20 in the last commit.

@mortenengen mortenengen changed the base branch from main to dev February 18, 2026 06:27
@mortenengen
Copy link
Member

mortenengen commented Feb 18, 2026

Thanks for the fix, @andrefmello91. I noticed now that the PR was setup to merge with main instead of dev, and therefore our pipeline with code format, linting and testing was not triggered. A local run of the pipeline shows that all is good except for one test of a large circular section. I will unfortunately not have time to look into this before the coming week, but please feel free to explore the issue further and propose a solution 😃

Please note that I have pushed a commit that formats the code and adds a note in the docstring of CircularGeometry about the number of points being rounded to the nearest multiple of 4. So before committing and pushing any changes, you should first pull.

@mortenengen mortenengen moved this from Changes requested 📝 to Review changes 🤔 in PR tracker Feb 18, 2026
@mortenengen mortenengen moved this from Review changes 🤔 to Changes requested 📝 in PR tracker Feb 18, 2026
@andrefmello91
Copy link
Author

Sorry for making the PR in the main branch.
I checked the error, but at the moment I could not figure what is causing it.

@mortenengen
Copy link
Member

Sorry for making the PR in the main branch. I checked the error, but at the moment I could not figure what is causing it.

No worries 😄 I will have a closer look the coming week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

Status: Changes requested 📝

Development

Successfully merging this pull request may close these issues.

2 participants