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

Configure predicates through class kwargs #81

Closed
florianfischer91 opened this issue Feb 2, 2022 · 4 comments · Fixed by #84
Closed

Configure predicates through class kwargs #81

florianfischer91 opened this issue Feb 2, 2022 · 4 comments · Fixed by #84

Comments

@florianfischer91
Copy link
Collaborator

Allow to configure predicates through class kwargs, allowing more elegant and compact code

class MyPredicate(Predicate, predicate_name="custom_name"):
    foo: int
    bar: int

would be equal to

class MyPredicate(Predicate):
    foo: int
    bar: int
    
    class Meta:
        name="custom_name"

Of course it would be much nicer to use name instead of predicate_name but currently in _PredicateMeta.__new__ name is already used as an argument

@daveraja
Copy link
Collaborator

daveraja commented Feb 11, 2022

It's a nice idea and this syntax would be very compact if it is possible to do.

I guess I have two concerns:

  1. What are the limitations of doing this? Is it possible to add arbitrary properties? Currently there is also "is_tuple" that can be specified (which is just an alternative to writing name=""). But in future I could imagine having other things; in particular I'd in interested to see if we could have a flag to make a given predicate usable in Pydantic.

  2. Even if it can be done, is it Pythonic? Pydantic uses a Config sub-class so is similar to clorm's Meta sub-class. Sqlalchemy uses special attributes like __tablename__ = "XXX". But, I am not aware of a project that uses the inheritance list.

In terms of name vs predicate_name. In _PredicateMeta.__new__, name is the class name and it is specified as a positional argument so I don't think that would be the reason you would have to use predicate_name instead of name. I would have thought that if this syntax is possible it would appear in the bases parameter somehow.

@florianfischer91
Copy link
Collaborator Author

You can basically add every Key-Value-Pair after the base class, as long as the key isn't already used as an argument in the __new__-Method of the Meta-Class. This means if we would rename the argument name in __new__ to f.i. __clorm_name__ we can also pass in name="custom_name"

All additional Key-Value-Pairs will be available in **kwargs in _PredicateMeta.__new__ and not in bases. They are not part of any inheritance structure.

You are right, Pydantic uses Config, but it is also possible to pass in Key-Value-Pairs. If you specifiy the same Config-Option in two places they are raising an TypeError see here

@daveraja
Copy link
Collaborator

Currently the predicate metaclass constructor is declared as __new__(meta, name, bases, dct).

In the context of metaclasses, does this mean that these first four variables have special meaning (although the variable names themselves can change), but then you can add arbitrary things on the end? So, something like: new(meta, class_name, bases, dct, name=None)`

That does seem pretty crazy (powerful but also potentially messed up)! I'll have to experiment with this at some point.

Anyway, if you think you can make it work, it does sound like a good compact way of specifying the mapping to the predicate name.

@florianfischer91
Copy link
Collaborator Author

I created a PR. Feel free to play around with it and let me know what you think :-)

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

Successfully merging a pull request may close this issue.

2 participants