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.

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

AttrDict fails when uniting regular dicts #640

Open
1 of 3 tasks
irm-codebase opened this issue Jul 17, 2024 · 4 comments
Open
1 of 3 tasks

AttrDict fails when uniting regular dicts #640

irm-codebase opened this issue Jul 17, 2024 · 4 comments
Labels
Milestone

Comments

@irm-codebase
Copy link
Contributor

What happened?

At the moment, AttrDict will fail to execute union if the merged object is a regular dictionary. This is despite the docstrings suggesting that this should be possible.

The cause is that we directly call other.keys_nested(), which is AttrDict exclusive. The solution is an additional check istype(other, AttrDict), and converting if False.

Which operating systems have you used?

  • macOS
  • Windows
  • Linux

Version

v0.7

Relevant log output

No response

@irm-codebase irm-codebase added bug v0.7 (upcoming) version 0.7 labels Jul 17, 2024
@brynpickering
Copy link
Member

We desperately want to move away from AttrDict (see #366) rather than constantly patching it. However, we haven't found an alternative that fits our needs.

@irm-codebase
Copy link
Contributor Author

@brynpickering I was looking at alternatives, and I found dotmap: https://github.com/drgrib/dotmap/tree/master

The only thing its missing is the "disallow overrides" feature we have, although I feel like we are always turning that off...
It has one disadvantage, though: it uses OrderedDict instead of just dict, which is slightly slower.

@brynpickering
Copy link
Member

Hmm yeah, the others I found also have only one downside each, so it's three options each with one problem making them unsuitable as drop-in replacements 😄

@irm-codebase
Copy link
Contributor Author

irm-codebase commented Jul 19, 2024

I'll try to come up with a nice working alternative later, so let's keep this issue open for now.

(subclass of Box with union_no_override or something)

@sjpfenninger sjpfenninger added this to the 0.7.x milestone Aug 6, 2024
@brynpickering brynpickering removed the v0.7 (upcoming) version 0.7 label Sep 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants