Skip to content

Move from attrs to dataclasses #200

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

Closed
wants to merge 2 commits into from

Conversation

hukkin
Copy link
Contributor

@hukkin hukkin commented Feb 19, 2022

BREAKING CHANGES

  • Token.as_dict -- Moved to using a simple dataclasses.asdict call. This means that many keyword arguments are removed from the method. Output is also slightly different (see .yml test regression files in this PR).
  • Token.copy -- Moved from attr.evolve to a simple copy.copy call. The stdlib copy.copy call does quite literally what the docstring of Token.copy says which is "return a shallow copy of the instance". Does attr.evolve do something extra? Is there a chance someone relies on this extra behavior? EDIT: Answering my own question: Without keyword arguments, attr.evolve seems doesn't seem to do anything that copy.copy doesn't. I don't think this is a breaking change.

@codecov
Copy link

codecov bot commented Feb 19, 2022

Codecov Report

Merging #200 (388f78c) into master (32097fb) will increase coverage by 0.00%.
The diff coverage is 98.07%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #200   +/-   ##
=======================================
  Coverage   96.14%   96.15%           
=======================================
  Files          61       62    +1     
  Lines        3244     3251    +7     
=======================================
+ Hits         3119     3126    +7     
  Misses        125      125           
Flag Coverage Δ
pytests 96.15% <98.07%> (+<0.01%) ⬆️

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

Impacted Files Coverage Δ
markdown_it/_compat.py 85.71% <85.71%> (ø)
markdown_it/ruler.py 89.74% <100.00%> (+0.08%) ⬆️
markdown_it/rules_inline/state_inline.py 97.91% <100.00%> (+0.02%) ⬆️
markdown_it/token.py 91.39% <100.00%> (+0.87%) ⬆️

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 32097fb...388f78c. Read the comment docs.

@chrisjsewell
Copy link
Member

@hukkin
Copy link
Contributor Author

hukkin commented Feb 19, 2022

Oh wasn't aware such API existed.

However, since we don't use the kwargs or replace anything, I think I'd prefer copy.copy for readability. What do you think?

@chrisjsewell
Copy link
Member

Heya @hukkin I went ahead and did this in #212, keeping outputs the same.

Didn't add the slots compat, because I was lazy. Feel free to add

@hukkin hukkin deleted the dataclasses branch April 15, 2022 06:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants