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

Small performance improvements for GeomBuilder._buildCoords #1462

Merged
merged 2 commits into from
Nov 10, 2024

Conversation

groutr
Copy link
Contributor

@groutr groutr commented Oct 29, 2024

Some minor improvements to help build coordinate lists a little more efficiently.

  1. Lift the ndim check out of the loop. This only needs to be checked once per call.
  2. Build tuples directly instead of creating a list and converting to tuples.

sgillies
sgillies previously approved these changes Nov 9, 2024
Copy link
Member

@sgillies sgillies left a comment

Choose a reason for hiding this comment

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

@groutr thank you! I'll backport to maint-1.10 after it's merged.

I made one suggestion.

Would a list comprehension be any faster?

fiona/_geometry.pyx Outdated Show resolved Hide resolved
@groutr
Copy link
Contributor Author

groutr commented Nov 10, 2024

I would be shocked if a list comprehension performed any different from a for-loop since they both generate identical C code.

@sgillies
Copy link
Member

I'll merge. Miniconda is failing us now. I've got a PR that moves us to micromamba.

@sgillies sgillies merged commit 864d532 into Toblerity:main Nov 10, 2024
6 of 9 checks passed
sgillies pushed a commit that referenced this pull request Nov 10, 2024
* Lift ndims check out of loop.

* Instantiate list at declaration.
@sgillies
Copy link
Member

Back ported to maint-1.10.

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