perf: faster canonicalize_version #993
Conversation
Signed-off-by: Henry Schreiner <henryfs@princeton.edu>
|
|
||
| if isinstance(version, str): | ||
| try: | ||
| version = str(Version(version)) |
There was a problem hiding this comment.
I think mypy had an issue without it? I know it was acting up here, it didn't like some variations.
Almost sure mypy gets confused in the except block without it. The other fix was to make this a new variable but then I have to add an else.
There was a problem hiding this comment.
Right, that makes sense, we're inside an isinstance(version, str) block, so MyPy is not going to be happy if you assign version a type which it isn't declared for, even if it's not actually needed.
I agree, I had to read up on this single dispatch pattern, it might be helpful when dealing with a large array of types, but we only have two types here. |
|
It's really nice for a specific type of design where you register every type you know how to handle. I use it in uproot-browser to register types we know how to plot, it's nice there. Not great for this, though, and I don't think I've seen a registered dispatch call the original like this before. |
I was looking at the Python 3.15 flamegraph for SpecifierSet's (using all
python-requiredon PyPI), and noticed a lot of time was spent inside this singledispatch. Dropping it simplifies the code and makes creating SpecifierSet's take 7% less time.