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

Class property #161

Open
fwkoch opened this issue Apr 27, 2017 · 3 comments
Open

Class property #161

fwkoch opened this issue Apr 27, 2017 · 3 comments

Comments

@fwkoch
Copy link
Contributor

fwkoch commented Apr 27, 2017

I think there is some value here? I don't think it's exceptionally common for a HasProperties instance to need a Class property value (as opposed to an Instance) but I think it's certainly a possibility.

class HasClass(properties.HasProperties):
    my_class = properties.Class('Some class...')

    def do_something(self):
        some_instance = self.my_class()
        ...

Traitlets has this, for what it's worth: https://github.com/ipython/traitlets/blob/4.3.2/traitlets/traitlets.py#L1527

@bsmithyman
Copy link
Member

So the thing that would keep this from being an Instance('...', type) property is just that that isn't really useful validation, right? Meaning the implementation is very similar to Instance, but just with an issubclass(prop, cls) check, rather than an isinstance(prop, cls) check?

Yeah, I can see that making sense, though I agree the use cases are probably a bit limited.

@fwkoch
Copy link
Contributor Author

fwkoch commented Apr 28, 2017

A draft of what I'm thinking is here: #163

Not sure if there's anything interesting we could be doing with HasProperties Classes. Also needs some thought about serialization...

@lheagy
Copy link
Contributor

lheagy commented Jul 3, 2019

This would really useful! Within SimPEG - we would use it for solvers. We use a Solver class that is instantiated during the computation of the fields or data, so I would like to set is as an uninstantiated class when we construct the object. Roughly, I would see something along the lines of

class Class(properties.Property):

    class_info = "a property that is an uninstantiated class"
    
    def validate(self, instance, value):
        if inspect.isclass(value) is False:
            extra = (
                "Expected an uninstantiated class. The provided value is not"
            )
            self.error(instance, value, TypeError, extra)
        self._parent_module = value.__module__
        return value

    def serializer(self, value, **kwargs):
        return "{}.{}".format(self._parent_module, value.__name__)

    def deserializer(self, value, **kwargs):
        name = value.split(".")
        try:
            module = sys.modules[".".join(name[:-1])]
        except KeyError:
            raise ImportError(
                "{} not found. Please install {}".format(
                    ".".join(value, name[0])
                )
            )
        return getattr(module, name[-1])

I took a crack at implementing the serialization / deserialization piece, and then pretty quickly ended up rather deep in properties, particularly with the behaviour of defaults, as there are quite a number of places where properties will try to instantiate callable values, but here, we would want to leave them uninstantiated.

On the Property (this one might be appropriate - it is straightforward to overwrite)

@default.setter
def default(self, value):
if callable(value):
self.validate(None, value())

Also in the sphinx docs (this seems redundant if it is already going through the default setter?)

if callable(self.default):
default_val = self.default()
default_str = 'new instance of {}'.format(
default_val.__class__.__name__
)

This one on HasProperties seems perhaps a bit too heavy handed as we would need to re-write the _reset method on the class where the Class property is being used:

if callable(val):
val = val()

Here is the current pass at a (rough) implementation within SimPEG:
https://github.com/simpeg/simpeg/blob/46a0527ed8c8d1594c108931449745eba256ed41/SimPEG/simulation.py#L40-L103

and the associated class that inherits HasProperties
https://github.com/simpeg/simpeg/blob/46a0527ed8c8d1594c108931449745eba256ed41/SimPEG/simulation.py#L142-L164

I would be happy to help iterate or test out a pr if that is of interest.

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

3 participants