Skip to content

[WIP] Avoid using __dict__#124

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

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

Conversation

@thedrow
Copy link
Copy Markdown
Contributor

@thedrow thedrow commented Apr 1, 2018

__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
Copy link
Copy Markdown

CLAassistant commented Apr 1, 2018

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
Copy link
Copy Markdown

coveralls commented Apr 1, 2018

Coverage Status

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

@thedrow
Copy link
Copy Markdown
Contributor Author

thedrow commented Apr 1, 2018

Before merging this I need to add tests.

@thedrow
Copy link
Copy Markdown
Contributor Author

thedrow commented Apr 2, 2018

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
Copy link
Copy Markdown
Contributor Author

thedrow commented Apr 2, 2018

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