Skip to content

Fix attribute setting #293

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

Merged
merged 1 commit into from
Nov 25, 2016
Merged

Conversation

filmor
Copy link
Member

@filmor filmor commented Nov 24, 2016

  • When modifying attributes on a class, PyType_Modified has to be called
  • The result of a lookup may be non-NULL without being a descriptor (e.g. if we
    set it manually using Class.attribute = 1)

Fixes issue #292

@filmor filmor force-pushed the fix-attribute-setting branch 2 times, most recently from e9757cc to 5608d98 Compare November 25, 2016 09:27
@filmor
Copy link
Member Author

filmor commented Nov 25, 2016

@denfromufa @vmuriart @tonyroberts Please review :)

- When modifying attributes on a class, PyType_Modified has to be called
- The result of a lookup may be non-NULL without being a descriptor (e.g. if we
  set it manually using `Class.attribute = 1`)
@filmor filmor force-pushed the fix-attribute-setting branch from 545ac02 to d96d0a5 Compare November 25, 2016 09:48
@filmor
Copy link
Member Author

filmor commented Nov 25, 2016

One minor (additional) change in the last push: I changed it to call PyType_Modified unconditionally, the docs are very slim on whether this is needed :/

@filmor
Copy link
Member Author

filmor commented Nov 25, 2016

@tonyroberts Is that a formal approval? ;)

@tonyroberts
Copy link
Contributor

tonyroberts commented Nov 25, 2016

@filmor haha I thought of it as a 'formal' approval, but I agree with the changes :)

@filmor filmor merged commit fb23dd1 into pythonnet:master Nov 25, 2016
@filmor filmor deleted the fix-attribute-setting branch November 25, 2016 12:00
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.

2 participants