Skip to content
This repository was archived by the owner on Oct 24, 2024. It is now read-only.

update using dictionary unpacking #213

Merged
merged 7 commits into from
Feb 10, 2023

Conversation

keewis
Copy link
Contributor

@keewis keewis commented Jan 27, 2023

Another option would be the python 3.9-only dict1 | dict2 (which involves fewer special chars and thus I find it easier to read).

@TomNicholas
Copy link
Member

Great, thank you @keewis !

Another option would be the python 3.9-only dict1 | dict2 (which involves fewer special chars and thus I find it easier to read).

Yes use that, and I'll merge this PR after I merge #214

@TomNicholas TomNicholas added the bug Something isn't working label Jan 27, 2023
@TomNicholas
Copy link
Member

@keewis that caused a bunch of TypeError: unsupported operand type(s) for |: 'Frozen' and 'dict'

@keewis
Copy link
Contributor Author

keewis commented Jan 27, 2023

I guess that's because Frozen has not been updated to implement __or__. So I guess this means we should revert that change for now? Otherwise we could just cast the Frozen object back to a mutable dict.

@TomNicholas
Copy link
Member

I guess that's because Frozen has not been updated to implement or.

Frozen is an xarray internals class, so we could add the __or__ method ourselves, but it seems more effort than its worth for this.

@keewis
Copy link
Contributor Author

keewis commented Feb 1, 2023

I agree, we can't make use of that for now. However, Frozen would be cast to dict anyways, so we could just do that explicitly?

Otherwise let's revert that change.

Edit: reverting seems easiest, so I did that

Copy link
Member

@TomNicholas TomNicholas left a comment

Choose a reason for hiding this comment

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

Feel free to merge!

@keewis keewis enabled auto-merge (squash) February 10, 2023 15:00
@keewis keewis merged commit 8a9b7bf into xarray-contrib:main Feb 10, 2023
@keewis keewis deleted the fix-update-override branch February 11, 2023 14:11
flamingbear pushed a commit to flamingbear/rewritten-datatree that referenced this pull request Jan 19, 2024
* merge two dictionaries using dictionary unpacking

* check that the fix actually worked

* use `|` to combine two dictionaries

* Revert "use `|` to combine two dictionaries"

This reverts commit ecfbbd55dc687ac5b2bb582cd3d29a4afc3608e4.

---------

Co-authored-by: Tom Nicholas <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants