Skip to content

Implement keys() and items() for AttrDict #1784

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged

Conversation

miguelgrinberg
Copy link
Collaborator

Fixes #1535

Copy link
Member

@pquentin pquentin left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM.

@miguelgrinberg miguelgrinberg force-pushed the document-update-with-innerdocs branch from 28761e4 to e043c6e Compare April 25, 2024 10:02
@miguelgrinberg miguelgrinberg merged commit a6ecf0b into elastic:main Apr 25, 2024
@miguelgrinberg miguelgrinberg added the backport 8.x Backport to 8.x label Apr 29, 2024
github-actions bot pushed a commit that referenced this pull request Apr 29, 2024
miguelgrinberg added a commit that referenced this pull request Apr 29, 2024
miguelgrinberg added a commit that referenced this pull request Apr 29, 2024
(cherry picked from commit a6ecf0b)

Co-authored-by: Miguel Grinberg <miguel.grinberg@gmail.com>
@kaos
Copy link

kaos commented May 28, 2024

This breaks uses where the wrapped data has "keys" or "items" in _d_ already.

That is, this used to work, but is now broken:

  a = utils.AttrDict({"items": [1, 2, 3]})
  assert a.items == [1, 2, 3]

@miguelgrinberg
Copy link
Collaborator Author

@kaos Thanks, I have missed this side effect. This limitation has existed in the InnerDoc class for a long time as well:

>>> from elasticsearch_dsl import InnerDoc, Text
>>> class MyDoc(InnerDoc):
...     items = Text()
...
>>> m = MyDoc(items="1,2,3")
>>> m.items
<bound method AttrDict.items of {'items': '1,2,3'}>

The workaround there was to use the dictionary syntax for these reserved names:

>>> m["items"]
'1,2,3'

So this is now also a limitation in AttrDict, and the same workaround applies.

@miguelgrinberg miguelgrinberg deleted the document-update-with-innerdocs branch May 28, 2024 14:16
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
backport 8.x Backport to 8.x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Document.update doesn't allow updates with InnerDoc
3 participants