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

Replace attrs with dataclasses #5284

Merged
merged 3 commits into from
Nov 25, 2020
Merged

Replace attrs with dataclasses #5284

merged 3 commits into from
Nov 25, 2020

Conversation

asvetlov
Copy link
Member

aiohttp 4.0 supports Python 3.7+.
There is no need to use attrs but better to switch to the standard dataclasses module.
attrs is great but the standard is even better.

The only downside is: for attrs you should use dataclasses.replace(ds, name=val) instead of attr.evolve(ds, name=val).
I think it is a good compromise.

@asvetlov asvetlov added the backport:skip Skip backport bot label Nov 24, 2020
@asvetlov asvetlov requested a review from webknjaz as a code owner November 24, 2020 12:02
@psf-chronographer psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label Nov 24, 2020
@codecov
Copy link

codecov bot commented Nov 24, 2020

Codecov Report

Merging #5284 (7949798) into master (7f73270) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5284      +/-   ##
==========================================
- Coverage   97.52%   97.52%   -0.01%     
==========================================
  Files          43       43              
  Lines        8784     8781       -3     
  Branches     1410     1410              
==========================================
- Hits         8567     8564       -3     
  Misses        103      103              
  Partials      114      114              
Flag Coverage Δ
unit 97.41% <100.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
aiohttp/client_reqrep.py 97.38% <100.00%> (-0.02%) ⬇️
aiohttp/connector.py 96.48% <100.00%> (ø)
aiohttp/helpers.py 96.78% <100.00%> (ø)
aiohttp/tracing.py 100.00% <100.00%> (ø)
aiohttp/web_request.py 97.60% <100.00%> (ø)
aiohttp/web_routedef.py 98.13% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7f73270...7949798. Read the comment docs.

@asvetlov asvetlov merged commit e51fb1f into master Nov 25, 2020
@asvetlov asvetlov deleted the dataclasses branch November 25, 2020 19:05
@gleb-chipiga
Copy link
Contributor

gleb-chipiga commented Nov 25, 2020

As far as I understand, dataclasses does not support __slots__, which were used with attrs. Have you decided to stop using __slots__?

@asvetlov
Copy link
Member Author

asvetlov commented Nov 26, 2020

I just don't care.
Slots save some memory.
The difference was huge for old Python, with a new compact dict (which split keys and values structure) implemented a gap is negligible.

commonism pushed a commit to commonism/aiohttp that referenced this pull request Apr 27, 2021
* Replace attrs with dataclasses

* Add CHANGES
commonism pushed a commit to commonism/aiohttp that referenced this pull request Apr 27, 2021
* Replace attrs with dataclasses

* Add CHANGES
@Tinche
Copy link

Tinche commented Jun 19, 2021

@asvetlov You are free to do as you like, obviously, but dataclasses aren't better than attrs. Their functionality is a strict subset; it's mostly for use within the standard library and if you don't have access to third party packages. Everyone using aiohttp obviously has access to third party packages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip Skip backport bot bot:chronographer:provided There is a change note present in this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants