cdgriffith / Box

Python dictionaries with advanced dot notation access
https://github.com/cdgriffith/Box/wiki
MIT License
2.61k stars 106 forks source link

Lock ability to create new fields #147

Open aronchick opened 4 years ago

aronchick commented 4 years ago

Is it possible to freeze new field creation via dot access without freezing the underlying fields to be immutable?

cdgriffith commented 4 years ago

I was actually thinking of this in a different way earlier this week. From the standpoint of having a Schema that a box had to adhere too. I personally decided against it as the best schema validation tool I know of for this would be jsonschema and it really has such simple operation it makes more sense to use it by itself than incorporate it into box.

However that is an interesting way to go about it is just not allowing more fields after initial creation. So the question then would be what about sub boxes, would they by default also be non-mutable, but their values be mutable? and then you could just upright replace a sub-box with a new one with different keys.

aronchick commented 4 years ago

Well, perhaps I'm not thinking about this properly :)

I'm inheriting from Box to create a subclass - MLBox - which is of a specific type. So, for the sake of argument, let's say I want to do the following (forgive the hacky code):

ml_dict = { 'schema': 'model_schema', 'framework': 'tensorflow', 'version':'2.0', 'model_package': { 'URI': 'https://s3.amazon.com/somebucket/model.pkl', 'model_sha': 'ea4963009b2c0f5b476984e024cbe66f5a8355d0'} }

# For a bunch of reasons, I need to split the instantiation of the Box from the setting of the dict from the loading of the dict
my_model_object = MLBox()

# Builds the empty schema - so all the keys above but set to "None"
my_model_object.set_schema('0.0.1', 'model_schema')

# Below works
my_model_object.framework = 'foo'

# Should throw keyerror, not create a new field
my_model_object.bar = 'qaz'

# below will overwrite anything already in the box
my_model_object.load(ml_dict)

Does this make sense?

As an aside, is there a blessed way to set all the fields AFTER the box has been instantiated? Or I'm just using self.merge_update(ml_dict) inside the .load(dict) function above, which feels fine, but wanted to make sure.

cdgriffith commented 4 years ago

That sounds like a reasonable way to approach it. It is not something outside the realm of possible, but would probably be a heavy addition / it's own inherited box type (like how ConfigBox or SBox are currently). Will admit not a high priority item myself, but would welcome PRs for it.

To achieve a similar thing now, I would still suggest using jsonschema so you could make said schema as you defined, and then before wherever the my_model_object will be used do a validate against it to make sure it's sane first.

As far as updating or setting fields the goal of Box is to be a pure dict drop in, so any of those methods should work as expected. Then merge_update is a "kinder" update that will merge sub Boxes/dicts together instead of overwriting what is already there. So that is more of a design choice. So yes, if that is the goal, that sounds like a good choice to me.

aronchick commented 4 years ago

Sounds good! Do you want me to leave this open as an enhancement?

And so, to be clear, is there anything I can do to allow for the freezing just of new field creation? (e.g. is there a method I should subclass in order to do this? or is it effectively impossible)?

cdgriffith commented 4 years ago

Yes please leave it open as an idea.

Easiest way I can think of would be to subclass box and the __setitem__ method to do a check of

if self._box_config["__created"] and key in self:
    raise BoxError("Cannot create new fields")
tikendraw commented 6 months ago

Just saying. Use Pydantic. It accepts dicts as input, parses as your intent and has configuration. I liked the idea of dict accessible with dot notation. It is now try to be pydantic.