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

Make Attribute instances immutable and slotted. #51

Merged
merged 3 commits into from Aug 13, 2016

Conversation

Projects
None yet
3 participants
@Tinche
Member

Tinche commented Aug 12, 2016

If this looks good to you, I'll add something to the changelog. Since the docs already define Attributes are read-only, this should be backwards compatible.

@codecov-io

This comment has been minimized.

codecov-io commented Aug 12, 2016

Current coverage is 100% (diff: 100%)

Merging #51 into master will not change coverage

@@           master   #51   diff @@
===================================
  Files           7     7          
  Lines         364   363     -1   
  Methods         0     0          
  Messages        0     0          
  Branches       86    86          
===================================
- Hits          364   363     -1   
  Misses          0     0          
  Partials        0     0          

Powered by Codecov. Last update a673093...987181a

@hynek

This comment has been minimized.

Member

hynek commented Aug 12, 2016

Yeah, I was always very explicit about this stuff being read-only so I’m not concerned. As for changelog, maybe just add the function names and the PR# to the existing “faster” entry?

Removed _fast_attrs_iterate (unnecessary).
Modify changelog entry.
@Tinche

This comment has been minimized.

Member

Tinche commented Aug 13, 2016

Ping.

@hynek

This comment has been minimized.

Member

hynek commented Aug 13, 2016

I’m conflicted about _fast_attrs_iterate. Maybe kill it off but don’t use fields in internal functions and just grab __attrs__attrs__ ? Because the new version is a slowdown due to the safety checks which we don’t need internally…

By which I mean: asdict → use fields. _convert__attrs_attrs__. Does that make sense?

@Tinche

This comment has been minimized.

Member

Tinche commented Aug 13, 2016

Agreed. (Pushed the change.)

fields() will give a proper error message if junk is passed in, but with a tiny slowdown. _convert can avoid the check. Other internal functions were already using the field directly.

@hynek hynek merged commit 46ac6b6 into python-attrs:master Aug 13, 2016

2 checks passed

codecov/project 100% (+0.00%) compared to a673093
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@hynek

This comment has been minimized.

Member

hynek commented Aug 13, 2016

Thanks!

@Tinche Tinche deleted the Tinche:feature/immutable-attributes branch Aug 13, 2016

@Tinche

This comment has been minimized.

Member

Tinche commented Aug 13, 2016

❇️❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment