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

Descriptors #2

Open
rochacbruno opened this issue Feb 22, 2022 · 1 comment
Open

Descriptors #2

rochacbruno opened this issue Feb 22, 2022 · 1 comment

Comments

@rochacbruno
Copy link

From readme I see

@dataclass    
class User(Obj):
    _conversion = {
        "id": int_conv,
        "language_code": string_conv,
        "username": string_conv,
    }

    id: int
    language_code: str
    username: str

Which makes me curious on why to define an argument named _conversion, is this a design decision for some reason?

What about implementing that using the descriptor protocol?

@dataclass    
class User(Obj):
    id: int = IntField()
    language_code: str = StringField()
    username: str = StringField()

And having all *Field to be implementation of descriptors such as:

class IntField:

    def __set_name__(self, owner, name):
        self.public_name = name
        self.private_name = '_' + name

    def __get__(self, obj, objtype=None):
        value = getattr(obj, self.private_name)
        logging.info('Accessing %r giving %r', self.public_name, value)
        return int(value)  # conversion here on reading

    def __set__(self, obj, value):
        logging.info('Updating %r to %r', self.public_name, value)
        setattr(obj, self.private_name, int(value))  # conversion here on writing
@monomonedula
Copy link
Owner

monomonedula commented Feb 23, 2022

The descriptors approach wasn't considered at the time of writing this library mostly because I wasn't an active user of the descriptors in general.

The idea seems kinda nice indeed. However there's a number of things to be considered if we are to implement this approach.

  • The use of descriptors will effectively make all fields optional for the class constructor. What I mean is something like this wouldn't be detected as a problem by the type checker anymore:
user = User()    # creating a user with no arguments
  • How do we implement the descriptors for fields with compound values (objects and arrays)?
  • Naming logic must be addressed too, since field name collision is possible (consider having two fields named foo and _foo in a model)

Could you also provide an example of a descriptor for some type that would actually require a conversion from/to JSON, as opposed to int?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants