Feature/slots #35

Tinche · 2016-03-17T09:19:29Z

No description provided.

Many more slot tests.

codecov-io · 2016-03-17T09:21:37Z

Current coverage is `100.00%`

Merging #35 into master will not affect coverage as of 8929cc7

@@            master     #35   diff @@
======================================
  Files            7       7       
  Stmts          343     361    +18
  Branches        80      86     +6
  Methods          0       0       
======================================
+ Hit            343     361    +18
  Partial          0       0       
  Missed           0       0

Review entire Coverage Diff as of 8929cc7

Powered by Codecov. Updated on successful CI builds.

tests/test_dark_magic.py

 foo = None


-@attr.s()


hynek · 2016-03-17T12:27:57Z

On a first scroll a few nits to keep you busy until I have actual time for a review :):

Please:

replace class_ through cls.
use my docstring style consistently (mostly test_slots.py)

Fix docstrings. Fix missing parens in a test.

hynek · 2016-03-21T09:33:54Z

OK one more fluff wish: could you please make all asserts to be assert expected == rv? I don’t Yoda in ifs either but I like to have the expected values lined up. :)

hynek · 2016-03-21T09:39:42Z

Just so we don’t forget:

document that slots=True replaces the class

src/attr/_make.py

+ cl_dict = dict(cl.__dict__)
+ cl_dict['__slots__'] = tuple(ca_list)
+ for ca_name in ca_list:
+ # It might not actually be in there, f.e. if using 'these'.


hynek · 2016-03-21T10:17:49Z

all in all this is exciting :)

Tinche · 2016-03-21T21:44:02Z

Pasting the answer link to a review question here so it doesn't get hidden by GitHub:

#35 (comment)

hynek · 2016-03-22T10:43:25Z

A question? Wrong link? Seemed more like a statement of facts. :)

Tinche · 2016-03-22T10:52:12Z

Ok, a clarification then. :)

hynek · 2016-03-22T12:40:27Z

Are you waiting for anything from me?

Tinche · 2016-03-22T12:41:36Z

Er no, I'm at work currently. Will fix the parametrization decorators tonight. That's all for now right?

hynek · 2016-03-22T12:52:48Z

That and docs. :)

FWIW, you can also use hypothesis for the decorators thing. I tend to introduce it into most of my current projects.

Tinche · 2016-03-22T12:55:02Z

Ok, about the docs. Obviously we need to document the slots argument to attr.s() and make_class, but we should probably have a small section somewhere on the benefits and drawbacks of slot classes. Any preference where?

hynek · 2016-03-22T14:32:27Z

I’d say examples. Bring an example and explain a bit. But you don’t have to explain slots completely. Only what’s special about our slots. Feel free to link a general description.

Tinche · 2016-03-22T23:30:59Z

Ok, I've added hypothesis in. The tests for py27 and pypy are actually failing for me currently, since hypothesis conditionally depends on enum34, and tox creates virtualenvs with ancient pip versions (1.5.6) which don't know to actually install enum34 for hypothesis. I will admit to not knowing how to fix this, and getting a little pissed off at tox for not having a mechanism to deal with this (or at least a discoverable mechanism).

I'll ping here when there's something to show for the docs, might be a few days.

hynek · 2016-03-23T10:33:18Z

You mean local? Because Travis passes just fine? Try tox -r :)

Tinche · 2016-03-23T10:45:23Z

I did try recreating, no dice. I guess my virtualenv package is just obsolete. Still frustrating to not be able to precisely tailor the test environment without hacks :/

hynek · 2016-03-23T11:02:45Z

I use pipsi for tox and keep my virtualenv/pip up2date, yeah. :)

Tinche · 2016-03-28T19:06:20Z

Just pushed some docs, for your inspection. Out of curiosity, do you get automatic notifications when I push new stuff?

hynek · 2016-03-29T08:26:51Z

Nope, always ping tickets when you do something. Getting a notification for each push would result in a lot of mails I guess but it would be nice if there’d be a button “notify maintainers” or something. :-/

hynek · 2016-03-29T08:28:28Z

Please do semantic new lines. :)

Tinche · 2016-03-29T18:36:22Z

Alright, am I doing it right? :)

hynek · 2016-03-29T20:20:57Z

I'll give more detailed feedback tomorrow but you slightly over-did it. :) No line feeds after commas please. :)

Tinche · 2016-03-29T20:45:54Z

https://raw.githubusercontent.com/brandon-rhodes/blog/master/texts/brandon/2012/one-sentence-per-line.rst seems to use line feeds after commas liberally. ¯_(ツ)_/¯

docs/examples.rst

+
+By default, instances of classes have a dictionary for attribute storage.
+This wastes space for objects having very few instance variables.
+The space consumption can become acute


hynek · 2016-03-30T08:06:07Z

Getting there! 🎉

* capitalize bullet items * replace bullet asterisks with dashes * indent bullet code fragments * join some lines

Tinche · 2016-03-30T20:55:23Z

Alright, I've incorporated your feedback to the best of my abilities :)

hynek · 2016-03-31T05:53:05Z

Change log entry and I think we can merge. :)

hynek · 2016-03-31T10:33:22Z

Nevermind, I’ll do it myself. Thanks for all the work!

Tinche · 2016-03-31T11:04:18Z

My pleasure, thanks for the reviews.

Fixes #35, #31

glyph · 2016-05-23T22:31:28Z

I just saw this land in a release. Once upon a time, @fijal told me that __slots__ were actually bad for PyPy. Summoning him to this ticket, I hope: did I even understand that correctly at the time? If so, is it still true? I want to use this because I love the added "no extra attributes" strictness, but I don't want to hurt performance there.

Tinche added 5 commits March 14, 2016 03:02

Initial slots implementation.

7b201eb

Handle some edge cases for slots.

26b7bff

Many more slot tests.

Flake8 fix.

bef8773

Fix reprs.

1d21609

Simplify slots class name handling.

7d0ce60

hynek reviewed Mar 17, 2016
View reviewed changes

tests/test_dark_magic.py

foo = None

@attr.s()

This comment was marked as spam.

Sign in to view

Rename class_ to cls.

37bc4b8

Fix docstrings. Fix missing parens in a test.

hynek reviewed Mar 21, 2016
View reviewed changes

src/attr/_make.py

cl_dict = dict(cl.__dict__)

cl_dict['__slots__'] = tuple(ca_list)

for ca_name in ca_list:

# It might not actually be in there, f.e. if using 'these'.

This comment was marked as spam.

Sign in to view

Tinche added 2 commits March 21, 2016 22:32

Flipped a bunch of asserts in test_slots.

c50d5aa

Consistency changes in _make.py.

fd62735

We hypothesizin'.

821e853

Add slots docs.

cfc42ef

hynek mentioned this pull request Mar 29, 2016

attr.ib(init=False, default=...) doesn't seem to work #36

Closed

Semantic newlines.

7818400

hynek reviewed Mar 30, 2016
View reviewed changes

docs/examples.rst

By default, instances of classes have a dictionary for attribute storage.

This wastes space for objects having very few instance variables.

The space consumption can become acute

This comment was marked as spam.

Sign in to view

Review feedback incorporated:

adb08a1

* capitalize bullet items * replace bullet asterisks with dashes * indent bullet code fragments * join some lines

hynek merged commit adb08a1 into python-attrs:master Mar 31, 2016

hynek added a commit that referenced this pull request Mar 31, 2016

CHANGELOG & small nits

c7842b8

Fixes #35, #31

hynek added a commit that referenced this pull request Mar 31, 2016

Merge branch 'pr/35'

871bb64

Fixes #35, #31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/slots #35

Feature/slots #35

Tinche commented Mar 17, 2016

codecov-io commented Mar 17, 2016

This comment was marked as spam.

hynek commented Mar 17, 2016

hynek commented Mar 21, 2016

hynek commented Mar 21, 2016

This comment was marked as spam.

hynek commented Mar 21, 2016

Tinche commented Mar 21, 2016

hynek commented Mar 22, 2016

Tinche commented Mar 22, 2016

hynek commented Mar 22, 2016

Tinche commented Mar 22, 2016

hynek commented Mar 22, 2016

Tinche commented Mar 22, 2016

hynek commented Mar 22, 2016

Tinche commented Mar 22, 2016

hynek commented Mar 23, 2016

Tinche commented Mar 23, 2016

hynek commented Mar 23, 2016

Tinche commented Mar 28, 2016

hynek commented Mar 29, 2016

hynek commented Mar 29, 2016

Tinche commented Mar 29, 2016

hynek commented Mar 29, 2016

Tinche commented Mar 29, 2016

This comment was marked as spam.

hynek commented Mar 30, 2016

Tinche commented Mar 30, 2016

hynek commented Mar 31, 2016

hynek commented Mar 31, 2016

Tinche commented Mar 31, 2016

glyph commented May 23, 2016

Feature/slots #35

Feature/slots #35

Conversation

Tinche commented Mar 17, 2016

codecov-io commented Mar 17, 2016

Current coverage is 100.00%

This comment was marked as spam.

hynek commented Mar 17, 2016

hynek commented Mar 21, 2016

hynek commented Mar 21, 2016

This comment was marked as spam.

hynek commented Mar 21, 2016

Tinche commented Mar 21, 2016

hynek commented Mar 22, 2016

Tinche commented Mar 22, 2016

hynek commented Mar 22, 2016

Tinche commented Mar 22, 2016

hynek commented Mar 22, 2016

Tinche commented Mar 22, 2016

hynek commented Mar 22, 2016

Tinche commented Mar 22, 2016

hynek commented Mar 23, 2016

Tinche commented Mar 23, 2016

hynek commented Mar 23, 2016

Tinche commented Mar 28, 2016

hynek commented Mar 29, 2016

hynek commented Mar 29, 2016

Tinche commented Mar 29, 2016

hynek commented Mar 29, 2016

Tinche commented Mar 29, 2016

This comment was marked as spam.

hynek commented Mar 30, 2016

Tinche commented Mar 30, 2016

hynek commented Mar 31, 2016

hynek commented Mar 31, 2016

Tinche commented Mar 31, 2016

glyph commented May 23, 2016

Current coverage is `100.00%`