ENH: improve performance of deprecate_positional#2283
ENH: improve performance of deprecate_positional#2283mwtoews merged 9 commits intoshapely:mainfrom Zaczero:main
Conversation
Pull Request Test Coverage Report for Build 14639955201Details
💛 - Coveralls |
There was a problem hiding this comment.
Thanks for your first contribution, it looks great! Have you verified the timing to see if it improves the bottlenecks?
Shapely is mostly untyped for no good reason, perhaps just a low priority. But for new(ish) methods, adding typing hints is welcome from my perspective.
I'll find some more time in the next few days to take deeper look at this PR, but it looks pretty close to ready.
No. I expect there to be no more measurable performance overhead (except for the typical decorator wrap overhead) as there is now only one |
|
@Zaczero thanks a lot for the PR! I had checked the timing and profile when checking #2280, so re-running that confirms your assumption there is now basically no overhead left: In [1]: import shapely
In [2]: %%timeit
...: for _ in range(1000):
...: coords = ((0., 0.), (0., 1.), (1., 1.), (1., 0.), (0., 0.))
...: polygon = shapely.Polygon(coords)
...:
21.7 ms ± 1.16 ms per loop (mean ± std. dev. of 7 runs, 100 loops each) # shapely 2.0.6
107 ms ± 8.73 ms per loop (mean ± std. dev. of 7 runs, 10 loops each) # shapely main
22.5 ms ± 1.52 ms per loop (mean ± std. dev. of 7 runs, 10 loops each) # this branch |
mwtoews
left a comment
There was a problem hiding this comment.
Finally got around to checking code, and indeed it restores the performance. Addition of missing tests is also very welcome. Thanks for the time assembling this PR, we should see this in a patch release anticipated soon!
Closes #2282
Closes #2280
It's my first time contributing here.
Potentially useful info for reviewers:
__code__docs reference (from Python 3.9): https://docs.python.org/3.9/library/inspect.htmlThis lets us skip the heavy inspect logic.
Another big optimization is the
n > warn_fromcheck and LRU cache of the messages, so people who don't resolve the deprecation warning aren't affected too much performance-wise.