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: Default box that doesn't raise key errors #67

Closed
Patrock opened this issue Dec 14, 2018 · 4 comments · Fixed by #145
Closed

Feature: Default box that doesn't raise key errors #67

Patrock opened this issue Dec 14, 2018 · 4 comments · Fixed by #145

Comments

@Patrock
Copy link

Patrock commented Dec 14, 2018

Hello,

Box is a nice little Dictionary Helper hich is working just fine most of the time. Currently i'm seeing a problem with the pop-method... When popping a non-exitent key with default_box=True i get a KeyError:

File "/mnt/d/projekte/python/envs/depmgmt/bin/box.py", line 592, in pop
del self[key]
File "/mnt/d/projekte/python/envs/depmgmt/bin/box.py", line 560, in delitem
super(Box, self).delitem(key)

Is this working as intended ? I would expect to get the given default value when the key is not existant ?

Thank you...

@cdgriffith
Copy link
Owner

Hi @Patrock

This is by design, to stay close to spec with the builtin defaultdict from collections

>>> from collections import defaultdict
>>> my_dict = defaultdict(int)
>>> my_dict['key1']
0

>>> my_dict
defaultdict(<class 'int'>, {'key1': 0})

>>> my_dict.pop('key2')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
KeyError: 'key2'

However I will leave this open for now to consider it as a feature request.

@Patrock
Copy link
Author

Patrock commented Dec 14, 2018 via email

@Patrock
Copy link
Author

Patrock commented Dec 14, 2018

Ok ... got it. Its the case with defaultdict... for me it seems that returning the default is the right way when it is advertised in the function. .?

@cdgriffith cdgriffith changed the title Problems with pop Feature: Default box that doesn't raise key errors Mar 12, 2020
@cdgriffith
Copy link
Owner

I think it would be best to add this as a full on box feature, that way both pop and del would not error if a key doesn't exist.

cdgriffith added a commit that referenced this issue Jul 12, 2020
* Adding support for msgpack converters `to_msgpack` and `from_msgpack`
* Adding support for comparision of `Box` to other boxes or dicts via the `-` sub operator #144 (thanks to Hitz)
* Adding support to `|` union boxes like will come default in Python 3.9 from PEP 0584
* Adding `mypy` type checking, `black` formatting and other checks on commit
* Adding new parameter `box_class` for cleaner inheritance #148 (thanks to David Aronchick)
* Adding `dotted` option for `keys` method to return box_dots style keys (thanks to ipcoder)
* Fixing box_dots to properly delete items from lists
* Fixing box_dots to properly find items with dots in their key
* Fixing that recast of subclassses of `Box` or `BoxList` were not fed box properties (thanks to Alexander Kapustin)
* Changing that sub boxes are always created to properly propagate settings and copy objects #150 (thanks to ipcoder)
* Changing that default_box will not raise key errors on `pop` #67 (thanks to Patrock)
* Changing `to_csv` and `from_csv` to have same string and filename options as all other transforms
* Changing back to no required external imports, instead have extra requires like [all] (thanks to wim glenn)
* Changing from putting all details in README.rst to a github wiki at https://github.com/cdgriffith/Box/wiki
* Changing `BoxList.box_class` to be stored in `BoxList.box_options` dict as `box_class`
* Changing `del` will raise `BoxKeyError`, subclass of both `KeyError` and `BoxError`
* Removing support for single level circular references
* Removing readthedocs generation
* Removing overrides for `keys`, `values` and `items` which will return views again
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants