Skip to content

Replace attrs with dataclasses#5284

Merged
asvetlov merged 3 commits intomasterfrom
dataclasses
Nov 25, 2020
Merged

Replace attrs with dataclasses#5284
asvetlov merged 3 commits intomasterfrom
dataclasses

Conversation

@asvetlov
Copy link
Copy Markdown
Member

@asvetlov asvetlov commented Nov 24, 2020

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
Copy Markdown

codecov bot commented Nov 24, 2020 •
edited
Loading

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.

derlih
derlih reviewed Nov 24, 2020
View reviewed changes
@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
Copy Markdown
Contributor

gleb-chipiga commented Nov 25, 2020 •
edited
Loading

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
Copy Markdown
Member Author

asvetlov commented Nov 26, 2020 •
edited
Loading

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
@asvetlov @commonism
Replace attrs with dataclasses (aio-libs#5284)
29466b7 
* Replace attrs with dataclasses

* Add CHANGES
commonism pushed a commit to commonism/aiohttp that referenced this pull request Apr 27, 2021
@asvetlov @commonism
Replace attrs with dataclasses (aio-libs#5284)
320ffeb 
* Replace attrs with dataclasses

* Add CHANGES
This was referenced Jun 18, 2021
Closed
Merged
@Tinche
Copy link
Copy Markdown

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.

@bdraco bdraco mentioned this pull request Oct 1, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@derlih derlih derlih left review comments

@webknjaz webknjaz Awaiting requested review from webknjaz webknjaz is a code owner

Assignees

No one assigned

Labels

backport:skip Skip backport bot bot:chronographer:provided There is a change note present in this PR

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@asvetlov @gleb-chipiga @Tinche @derlih