Skip to content

Fix evolve (cont)#182

Merged
hynek merged 9 commits intomasterfrom
bug/fix-175
May 4, 2017
Merged

Fix evolve (cont)#182
hynek merged 9 commits intomasterfrom
bug/fix-175

Conversation

@Tinche
Copy link
Copy Markdown
Member

@Tinche Tinche commented May 2, 2017

This is a continuation of #176.

cournape and others added 4 commits April 3, 2017 16:40
@cournape
BUG: fix evolve behaviour when a validator fails.
e24ef7f 
TypeError may be coming from many sources, and it is not correct to
assume it always comes from a non-existing attr member as the evolve
codes now. Instead, we now check specifically for this case when raising
a custom error, and just re-raise in other cases.
@cournape
STY: fix flake8 violations.
a6023aa
@cournape
ENH: don't recompute name -> attr mapping.
82d610f
@Tinche
Make evolve more like __init__.
5f0b3a0
@codecov
Copy link
Copy Markdown

codecov Bot commented May 2, 2017

Codecov Report

Merging #182 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #182   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           8      8           
  Lines         551    551           
  Branches      122    122           
=====================================
  Hits          551    551
Impacted Files Coverage Δ
src/attr/_funcs.py 100% <100%> (ø) ⬆️

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 beca344...5f0b3a0. Read the comment docs.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 2, 2017 •
edited
Loading

Codecov Report

Merging #182 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #182   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           8      8           
  Lines         551    546    -5     
  Branches      122    121    -1     
=====================================
- Hits          551    546    -5
Impacted Files Coverage Δ
src/attr/_funcs.py 100% <100%> (ø) ⬆️

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 beca344...eb1465a. Read the comment docs.

@Tinche
Copy link
Copy Markdown
Member Author

Tinche commented May 3, 2017

One minor nitpick. In case of a non-existent attribute evolve will raise AttrsAttributeNotFoundError.

I suggest this is unnecessary. __init__ doesn't raise a special attrs exception, just a TypeError, let's let evolve do the same (since it's kind of a pseudo __init__ anyway).

The code will be simpler, and we can avoid a confusing error message like this:

@attr.s
class A:
    _a = attr.ib()

>>> attr.evolve(A(1), _a=2)
attr.exceptions.AttrsAttributeNotFoundError: _a is not an attrs attribute on <class '__main__.A'>.

@Tinche Tinche requested review from glyph and hynek May 3, 2017 00:03
@Tinche Tinche mentioned this pull request May 3, 2017
Closed
hynek
hynek requested changes May 3, 2017
View reviewed changes
Copy link
Copy Markdown
Member

@hynek hynek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few minor doc things but otherwise LGTM. If @glyph ’s happy too, we can close this can of worms. :)

Comment thread tests/test_funcs.py Outdated

def test_validator_failure(self):
"""
Make sure we don't swallow TypeError when validation fails within

This comment was marked as spam.

Sign in to view

Comment thread tests/test_funcs.py Outdated

def test_private(self):
"""
evolve() should act as `__init__` with regards to private attributes.

This comment was marked as spam.

Sign in to view

Comment thread src/attr/_funcs.py Outdated
"{k} is not an attrs attribute on {cl}.".format(k=k, cl=cls))
for name in changes:
if getattr(attrs, name, None) is None:
k = exc.args[0].split("'")[1]

This comment was marked as spam.

Sign in to view

@glyph
Copy link
Copy Markdown
Contributor

glyph commented May 3, 2017

I've only had time to glance at it (I definitely won't have time to do a full review), but I generally agree. This should probably land.

@Tinche
Copy link
Copy Markdown
Member Author

Tinche commented May 3, 2017

Review fixes done!

@hynek hynek merged commit b79a49e into master May 4, 2017
@hynek hynek deleted the bug/fix-175 branch May 4, 2017 11:20
@Tinche
Copy link
Copy Markdown
Member Author

Tinche commented May 4, 2017

🎉

@hynek hynek mentioned this pull request May 10, 2017
Closed
hynek pushed a commit to adetokunbo/attrs that referenced this pull request May 10, 2017
@Tinche @hynek
Fix evolve (cont) (python-attrs#182)
8bd78e9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hynek hynek hynek approved these changes

@glyph glyph Awaiting requested review from glyph

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Tinche @glyph @hynek @cournape