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

__add__ and __or__ operators for frozen boxes #238

Closed
ptrba opened this issue Dec 28, 2022 · 3 comments
Closed

__add__ and __or__ operators for frozen boxes #238

ptrba opened this issue Dec 28, 2022 · 3 comments

Comments

@ptrba
Copy link

ptrba commented Dec 28, 2022

Frozen boxes are great for enforcing a functional style of programming at runtime. But the implementation of Box.__add__ assumes that the left-hand side is non-frozen.

The following piece of code fails in box/box.py:274: box.box.Box.__add__ with box.exceptions.BoxError: Box is frozen

foo = box.Box(frozen_box=True)
foobar = foo + {"bar": 1}

As far as I can judge, there is no reason to restrict __add__ (and __or__) operations to non-frozen boxes. The implementation of Box.__add__ creates a copy of the input box before merging/updating, thus the operations leave the input boxes unmodified.

This implementation of Box.__or__ would solve the issue:

...
new_box = other.copy()
new_box._box_config["frozen_box"] = False
new_box.update(other)
new_box._box_config["frozen_box"] = other._box_config["frozen_box"]
...

I've made a quick check that this works for flat and nested structures. But there might be some unintended side effects for nested structures. But this would be an issue even for non-frozen boxes. I guess most programmers will assume that writing foobar = foo + {"bar": 1} won't mutate foo.

What are the objections against frozen boxes in Box.__add__ (Box.__or__)? And, how should this be implemented?

@cdgriffith
Copy link
Owner

noobjection

Simply never thought of that case.

I think the way you have it is nearly same as I would (would just use self._box_config["frozen_box"] instead of other.)

cdgriffith added a commit that referenced this issue Jan 28, 2023
@cdgriffith
Copy link
Owner

Adding this feature in Box 7! Please test and give feedback if possible pip install python-box[all]~=7.0.0rc0

cdgriffith added a commit that referenced this issue Feb 4, 2023
* Adding #169 default functions with the box_instance and key parameter (thanks to Коптев Роман Викторович)
* Adding #170 Be able to initialize with a flattened dict - by using DDBox (thanks to Ash A.)
* Adding #192 box_dots treats all keys with periods in them as separate keys (thanks to Rexbard)
* Adding #211 support for properties and setters in subclasses (thanks to Serge Lu and David Aronchick)
* Adding #226 namespace to track changes to the box (thanks to Jacob Hayes)
* Adding #236 iPython detection to prevent adding attribute lookup words (thanks to Nishikant Parmar)
* Adding #238 allow ``|`` and ``+`` for frozen boxes (thanks to Peter B)
* Adding new DDBox class (Default Dots Box) that is a subclass of SBox
* Fixing #235 how ``|`` and ``+`` updates were performed for right operations (thanks to aviveh21)
* Fixing #234 typos (thanks to Martin Schorfmann)
* Fixing no implicit optionals with type hinting
@cdgriffith
Copy link
Owner

Added in 7.0.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants