-
-
Notifications
You must be signed in to change notification settings - Fork 395
Improve performance of asdict in the common case #1463
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
base: main
Are you sure you want to change the base?
Conversation
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
In preparation for #1463 Co-authored-by: Pieter Eendebak <883786+eendebakpt@users.noreply.github.com>
In preparation for #1463 Co-authored-by: Pieter Eendebak <883786+eendebakpt@users.noreply.github.com>
CodSpeed Performance ReportMerging #1463 will improve performances by ×2.6Comparing Summary
Benchmarks breakdown
|
@@ -66,6 +66,20 @@ def test_recurse(self, C, dict_class): | |||
C(C(1, 2), C(3, 4)), dict_factory=dict_class | |||
) | |||
|
|||
def test_non_atomic_types(self, C): | |||
""" | |||
Test subclasses of atomic types |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't conform with our docstring standard and proves why it's important :)
What exactly is tested here? (c.f. https://hynek.me/articles/document-your-tests/ for more context)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The method name and docstring indeed did not match, I updated the name.
I was trying to test the else
branch inside the recursive part of the asdict
implementation. The branch covers non-atomic objects minus some container types. I could add some more tests (e.g. fraction.Fraction
) if needed.
Summary
We improve performance of
attrs.asdict
by:issubclass
instead ofisinstance
for various type checksBenchmark:
Test script
Notes:
astuple
. That is left to a separate PR though. Also see attrs.astuple() leaves performance on the table compared to operator.attrgetter #1129Pull Request Check List
main
branch – use a separate branch!Our CI fails if coverage is not 100%.
.pyi
).tests/typing_example.py
.attr/__init__.pyi
, they've also been re-imported inattrs/__init__.pyi
.docs/api.rst
by hand.@attr.s()
and@attrs.define()
have to be added by hand too.versionadded
,versionchanged
, ordeprecated
directives.The next version is the second number in the current release + 1.
The first number represents the current year.
So if the current version on PyPI is 22.2.0, the next version is gonna be 22.3.0.
If the next version is the first in the new year, it'll be 23.1.0.
attrs.define()
andattr.s()
, you have to add version directives to both..rst
and.md
files is written using semantic newlines.changelog.d
.