dictionary-named methods are raising a validation exception

[toumorokoshi]

There's currently a bug where naming fields of a model after the dictionary methods will raise an incorrect DataError during validation (see unit test or toumorokoshi/transmute-core#20)

Looking at the documentation, the atoms() method shouldn't support anything besides a mapping anyway. This builds off of that assumption, and removes the getattr call.

[lkraider]

Indeed this will fix this test case, but users that override other mapping methods (ex: "keys") will still break (relevant: #375). Also, overriding "get" will break other things (ex: m.atoms() will not work anymore to retrieve data), so you might get Undefined values depending on the case, as it will break this current assumption:

convert(m._schema, m) == convert(m._schema, m._data)

I could go ahead and merge this, but not sure a new release is warranted until a more general fix is achieved.

[toumorokoshi]

@lkraider thanks, that ticket provided a lot of context. I see why this doesn't solve the larger collisions that occur between field names and the reserved method names of the Mapping type itself.

It sounds like there's a lot of work to get to the point of separating the two apart (and a significant part backwards-incompatible). I can avoid dealing with this head on by using serialize_name and deserialize_from for now.

I would still argue this patch should be considered. It's a minimal change that allows better compatibility with the set, at least as far as the core schematics serialize logic is concerned. I actually think embarking on the larger solution is a good long-term goal, but I personally don't have the bandwidth to tackle that right now.

I've made a change to rely on __getitem__ instead of get. This ensures that overriding any public method will not break schematics ability to serialize / deserialize.

[lkraider]

👍 from me after these few changes

[lkraider]

Please don't change whitespace formatting in this patch.

[lkraider]

See first comment.

[lkraider]

Please add a test for nested ModelType that override mapping methods too:

def test_nested_model_override_mapping_methods():

    class Nested(Model):
        items, values, get, keys = IntType(), IntType(), IntType(), IntType()

    class Root(Model):
        keys = ModelType(Nested)

    root = Root({"keys": {"items": 1, "values": 1, "get": 1, "keys": 1}})
    root.validate()
    assert root.keys.items == 1

[lkraider]

I put forward a patch on master that fixes the reliance on keys iteration. You can amend the test-cases to include a keys attribute.

[toumorokoshi]

Sounds good! Addressed all feedback, I believe. Thanks for fixing up the keys behavior as well!

Also just rebased on top of your changes, hope that's ok. not sure how that affects the existing CR.

[lkraider]

Let me know if you have issues with the latest master. I will prepare a 2.0.1 release including this fixes later in the week.

[lkraider]

FYI: v2.0.1 was just released to PyPI and contains these fixes.

[toumorokoshi]

works for me, thanks!