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

Feature Request: add toggle for Default Boxes to not create missing sub-boxes during get requests #164

Closed
ipcoder opened this issue Aug 10, 2020 · 5 comments

Comments

@ipcoder
Copy link

ipcoder commented Aug 10, 2020

Hi
I know there were significant changes in 5+, so I could have missed that, but I thought
reading from not existing item does not creates it.

So, my question is, if the code below works as intended?

import box
print(box.__version__)

b = box.Box(default_box=True)
print(b)
print(b.z)
print(b)

Produces:

5.1.0
{}
{}
{'z': {}}

Before I could have default_box to create items on assignment, but not on reading.
For me it seems quite a problematic behavior.

@cdgriffith
Copy link
Owner

That was actually changed in 4.1 IIRC https://github.com/cdgriffith/Box/wiki/Types-of-Boxes

This was done to match how defaultdict operates:

>>> from collections import defaultdict
>>> a = defaultdict(defaultdict)
>>> a['a']
defaultdict(None, {})
>>> a
defaultdict(<class 'collections.defaultdict'>, {'a': defaultdict(None, {})})

@ipcoder
Copy link
Author

ipcoder commented Sep 17, 2020

Hi, I tried to work with that, but its appears quite problematic in certain cases.
For example in IPython under the hood creates new fields in the data structure casing errors.

Also that has rendered a lot of code based on the old version (<4.1) as buggy.
Assuming there is a real incentive to support this defaultdict behaviour - can it be optional?

As an example please consider this use case:

par = Box(default_box=True)
par.clear_cnf.mask_disp = False
par.clear_cnf.enable = True

Initialization like that, which requires default_box=True

In interactive environment there are several scenarios when not existing field is accessed:

  • the framework like Jupyter does that by itself.
  • user may make typing error when reading from the Box.
    In all those cases invalid fields silently appear leading to errors in random places when unexpected fields are passed to functions, etc.

So having the old behavior as option would be quite helpful.

@cdgriffith
Copy link
Owner

I'm going to label this a feature request for that like suggested, because using an interpreter interactively is something I want to specifically target with Box's usability, and iPython is probably the best out out there.

It is a "medium" size feature request, as thankfully if implemented correctly won't break backwards compatibility, so can be a minor version change. However it is not a simple toggle, as the current process for default boxs resolves around that behavior.

Thank you for the suggestion and clearly laying out the problem space!

@cdgriffith cdgriffith changed the title __getitem__ creates items? Feature Request: add toggle for Default Boxes to not create missing sub-boxes during get requests Sep 18, 2020
cdgriffith added a commit that referenced this issue Feb 14, 2022
… variable on get request (thanks to ipcoder)

* Changing #123 box_dots to convert ints and bytes as needed (thanks to Marcelo Huerta)
* Changing #192 box_dots and box_default to split and create sub dictionaries as needed for insertion (thanks to Rexbard)
cdgriffith added a commit that referenced this issue Feb 19, 2022
… variable on get request (thanks to ipcoder)
cdgriffith added a commit that referenced this issue Mar 15, 2022
* Adding Cython support to greatly speed up normal Box operations on supported systems
* Adding #161 support for access box dots with `get` and checking with `in` (thanks to scott-createplay)
* Adding #183 support for all allowed character sets (thanks to Giulio Malventi)
* Adding #196 support for sliceable boxes (thanks to Dias)
* Adding #164 default_box_create_on_get toggle to disable setting box variable on get request (thanks to ipcoder)
* Changing #208 __repr__ to produce `eval`-able text (thanks to Jeff Robbins)
* Changing #215 support ruamel.yaml new syntax (thanks to Ivan Pepelnjak)
* Changing `update` and `merge_update` to not use a keyword that could cause issues in rare circumstances
* Changing internal `_safe_key` logic to be twice as fast
* Removing support for ruamel.yaml < 0.17
@cdgriffith
Copy link
Owner

Box 6 has been released with this added, thanks for opening the issue! default_box_create_on_get can now be set to False.

@ipcoder
Copy link
Author

ipcoder commented Dec 20, 2022

Hi. I am sorry to be a little late on this topis.
Thank you for addressing this issue, however I think that current behavior of default_box is lacking consistency.

There is a common use case of setting up a "deep" field, so let see how it plays out in different configurations.

As a special note, lets us take into account a special case interactive environments:
(PyCharm Debugger, Jupyter Notebook, or just user using completion for wrong field in ipython prompt ...)
which may dynamically query (get) for not-existing fields and unwillingly create them

b = Box(default_box=True)
b.x.y = 10  #  assignment works 

b = Box(default_box=True)
b.x.y          # produces new field (could be "unwillingly" - undesired)

So default_box_create_on_get is supposed to solve this unwilling creation, but then we loose the assignment:

b = Box(default_box=True, default_box_create_on_get=False)
b.x.y = 10  # does NOT work, b remains empty, contrary to expected from such code

b = Box(default_box=True, default_box_create_on_get=False)
b.x.y          # no new fields (desired)

Please let me know if I am missing something.

As a side note, I believe I understand possible design difficulties to support those scenarios.
We can discuss possible solutions if you want.

Thanks again.

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