Skip to content

[WIP] Avoid using __dict__#124

Open
thedrow wants to merge 5 commits into
uber:masterfrom
thedrow:patch-1
Open

[WIP] Avoid using __dict__#124
thedrow wants to merge 5 commits into
uber:masterfrom
thedrow:patch-1

Conversation

@thedrow

@thedrow thedrow commented Apr 1, 2018

Copy link
Copy Markdown
Contributor

__dict__ is an implementation detail. We should avoid using it.

Fixes #93.

__dict__ is an implementation detail. We should avoid using it.

Fixes uber#93.
@CLAassistant

CLAassistant commented Apr 1, 2018

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 97.689% when pulling c8b0476 on thedrow:patch-1 into e3814d2 on uber:master.

1 similar comment
@coveralls

coveralls commented Apr 1, 2018

Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 97.689% when pulling c8b0476 on thedrow:patch-1 into e3814d2 on uber:master.

@thedrow

thedrow commented Apr 1, 2018

Copy link
Copy Markdown
Contributor Author

Before merging this I need to add tests.

@thedrow

thedrow commented Apr 2, 2018

Copy link
Copy Markdown
Contributor Author

So it turns out this library indeed doesn't support partial doubles.
We can fix the tests because now we get a proper error.

I'd appreciate a code review before going any further.

@toddsifleet

Copy link
Copy Markdown
Contributor

@thedrow what do you mean by "indeed doesn't support partial doubles"? Do you have an example?

@toddsifleet

Copy link
Copy Markdown
Contributor

I'm not sure this will solve the problem, because we get errors like
AttributeError: 'UserWithSlots' object attribute 'arbitrary_callable' is read-only

I have to think more about how to support objects with __slots__

@thedrow

thedrow commented Apr 2, 2018

Copy link
Copy Markdown
Contributor Author

Exactly my point but at least with this PR we get a clearer error message.

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.

expect/allow not working for object with __slots__

4 participants