Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Can AttributeAnnotations set __parent__? #11

Closed
jamadden opened this issue Aug 9, 2017 · 4 comments · Fixed by #13
Closed

Can AttributeAnnotations set __parent__? #11

jamadden opened this issue Aug 9, 2017 · 4 comments · Fixed by #13
Labels
enhancement

Comments

@jamadden
Copy link
Member

jamadden commented Aug 9, 2017 •
edited
Loading

This would make it compatible with the IConnection adapter provided by zope.keyreference (assuming the object it annotates is persistent), which could simplify some use cases. AttributeAnnotations is itself not persistent, though the BTree it contains is.

For pickle compatibility this could be a property that reads self.obj.

Are there any downsides to making this generally the case?

(This comes out of a discussion in a private repository.)
@jamadden jamadden mentioned this issue Aug 9, 2017
Merged
@jamadden
Copy link
Member Author

jamadden commented Aug 9, 2017 •
edited by icemac
Loading

I'll note that zope.principalannotation does this by default.

@icemac
Copy link
Member

icemac commented Aug 23, 2017

I'll note that zope.principalannotation does this by default.

But the Annotations object used there is a subclass of Persistent and Location.

I am not sure what you want to change. Could you sketch it on a branch? I think it is much easier to think about existing code.

jamadden added a commit that referenced this issue Aug 24, 2017
@jamadden
Make AttributeAnnotations have a __parent__
45d1fd6 
Fixes #11 in a BWC way using a property. This makes it more like
zope.principalannotation (though I didn't change the interface here).

Also some doc fixes I noticed when I was fixing up the doctest
snippets (I originally had this implementing ``ILocation`` with a
`__name__` of `__annotations__` but I realized that didn't really make
any sense.)
jamadden added a commit that referenced this issue Aug 24, 2017
@jamadden
Make AttributeAnnotations have a __parent__
72b9f2b 
Fixes #11 in a BWC way using a property. This makes it more like
zope.principalannotation (though I didn't change the interface here).

Also some doc fixes I noticed when I was fixing up the doctest
snippets (I originally had this implementing ``ILocation`` with a
`__name__` of `__annotations__` but I realized that didn't really make
any sense.)
@jamadden jamadden mentioned this issue Aug 24, 2017
Merged
@jamadden
Copy link
Member Author

Sure, #13. It's very simple, it isn't even a new field.

jamadden added a commit that referenced this issue Aug 24, 2017
@jamadden
Make AttributeAnnotations have a __parent__
2084a5c 
Fixes #11 in a BWC way using a property. This makes it more like
zope.principalannotation (though I didn't change the interface here).

Also some doc fixes I noticed when I was fixing up the doctest
snippets (I originally had this implementing ``ILocation`` with a
`__name__` of `__annotations__` but I realized that didn't really make
any sense.)
@icemac
Copy link
Member

icemac commented Sep 7, 2017

I did not think it was that easy, but a line of code tells more than a hundred words. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Make AttributeAnnotations have a __parent__ zopefoundation/zope.annotation
2 participants
@icemac @jamadden