Allow both typed and raw subclasses of MapAttribute

[bugeats]

Imagine me wanting a simple key/value attribute. Ok no prob just use MapAttribute -- just as long as you don't subclass MapAttribute -- in which case the entire behavior is flipped upside down. Wah?

I find this documentation to be completely impenetrable:

PynamoDB/pynamodb/attributes.py

Line 573 in 4462874

class MapAttribute(Attribute, AttributeContainer):

I read it 10 times at least, and I still don't know what to do. Regardless if that design is here to stay or not, I humbly suggest adding something like KeyValueAttribute to hide the pain from dummies like me.

[jpinner-lyft]

👋 can you add some example code to show what you're trying to do when subclassing? In the subclass' init method, did you call super init?

[bugeats]

Oh hi. 👋 Yeah. I'm just sub-classing to support a simple framework for aggregation of schema definitions. So it might look like this:

from pynamodb.attributes import MapAttribute
from pynamodb.models import Model


class MyKeyValueMapAttribute(MapAttribute):
    @classmethod
    def to_schema(cls):
        return {
            'type': 'object'
        }


class MyModel(Model):
    class Meta:
        host = '...'
        table_name = '...'

    bucket = MyKeyValueMapAttribute(null=True, attr_name='bucket', default={})

I'm not trying to make a unique record, just trying to extend the base MapAttribute with some custom methods.

EDIT: no, no super init

[jpinner-lyft]

So MapAttributes as you've found are unfortunately very "special" -- they have a dual behavior that depends on both where they are defined and how they are accessed :(

The short answer is I think you need to override the is_raw class method in your subclass to return True.

Edit: is_raw is used to distinguish between MapAttributes that are "simple" maps versus those that strictly define the attributes inside of them.

[bugeats]

Hey is_raw class method did the trick, thanks!

I was trying to be careful with the title of this issue, since I do think special behavior is the core of the problem. May we keep the ticket open to represent the hope of creating Attribute classes with more clearly defined boundaries in their behavior?

Again I think teasing out a KeyValueAttribute would go a long way, whether or not the core architecture of "dual behavior that depends on both where they are defined and how they are accessed" is reconsidered.

Thanks again.

[jpinner-lyft]

The addition of "typed" subclasses. along with the use of class attributes in expressions, certainly caused complexity in the implementation. A lot was done to ensure backwards compatibility as the feature set grew.

I don't think there was ever any thought to allowing users to subclass map attributes while keeping the "raw" behavior. All subclasses were assumed to be typed bags.

Perhaps a better way to phrase this would be as a feature request?

"Allow both typed and raw subclasses of MapAttribute."

cc/ @ikonst @jmphilli @garrettheel

[ikonst]

Splitting the "raw" behavior out of MapAttribute would probably make the type annotation simpler too.

[garrettheel]

@jpinner-lyft @ikonst agreed that subclassing while maintaining the "raw" behavior is something we should support. Feel free to file this as a bug or feature request

[jpinner-lyft]

#868 might actually address this

by subclassing DynamicMapAttribute you always get a "raw" version as long as you don't define any attributes