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

Extending a class from another package #317

Open
hadley opened this issue Jul 19, 2023 · 6 comments · May be fixed by #341
Open

Extending a class from another package #317

hadley opened this issue Jul 19, 2023 · 6 comments · May be fixed by #341
Milestone

Comments

@hadley
Copy link
Member

hadley commented Jul 19, 2023

Currently if you extend a class from another package, the parent class will be captured at build-time. So if that package is later updated, and your package is not re-built, it's possible for two inconsistent versions of a class to exist. (The methods will be fine since they're already registered dynamically if needed, but the properties and validator might not).

We might be able to fix this and #250 at the same time by creating something like new_external_class() which contains just enough information to perform minimal static checks with the actual invocation always happening at run time. This will require some careful thought and analysis.

@lawremi
Copy link
Collaborator

lawremi commented Aug 14, 2023

Could we detect when this is happening, just by comparing the namespace/package of the two classes, and perform some sort of lazy updating behind the scenes? There would then be no need for something like new_external_class(). Not being familiar with how things are implemented, I'm not sure about feasibility, but I could imagine tracking some sort of timestamp or hash that minimizes the overhead of this.

@hadley
Copy link
Member Author

hadley commented Aug 15, 2023

We could also use the package field of the class object to do this automatically.

Regardless, we may still need some optional additional way of lazily registering classes in case you want to extend a class from a suggested package or for #250. But hopefully we can reserve new_external_class() for rare cases, like we now have for new_external_generic() .

@hadley
Copy link
Member Author

hadley commented Sep 9, 2023

I think the easiest way to start prototyping this is to allow parent to be a zero-argument function that you can call to dynamically get the parent class, and then see what breaks. Once that's complete we can figure out a more user friendly interface.

Ways in which the parent is currently used:

  • To determine the full set of properties. I think it's ok to keep this static — if a foreign-package superclass adds new properties, you won't be able to access them until you rebuild. But the existing code can't already use them so this shouldn't affect behaviour. (Except if the superclass adds a property with the same name as your class; but that's a problem no matter what you do because adding superclass properties is a potential breaking change).
  • In the default constructor. But we already branch based on whether the class is base/S3/S7 so we could easily add a new branch for a function call.
  • Stored in the parent attribute, which is used only in a handful of places.

@hadley
Copy link
Member Author

hadley commented Sep 9, 2023

I think we want this to be purely dynamic; another option would be to somehow replace a static class definition at package load time but I think that would require either walking over every object in the package environment or replacing the parent class definition with a reference type where we could modify in one place.

@hadley hadley linked a pull request Sep 9, 2023 that will close this issue
@hadley
Copy link
Member Author

hadley commented Sep 14, 2023

Ways that a parent class might change (to help me think about the problem):

How could a parent class change?

  • It could add a new property:
  • It could remove a property
  • It could make a property more permissive
  • It could make a property more restrictive
  • It could change the property class
  • It could change the property default
  • It could change its constructor
  • It could make the validator more strict
  • It could make the validator more permissive
  • It could change its parent
  • It could change its name/package
  • It could change its abstract status

@ltierney
Copy link
Collaborator

ltierney commented Sep 14, 2023 via email

@hadley hadley added this to the v0.2.0 milestone Oct 17, 2023
@t-kalinowski t-kalinowski modified the milestones: v0.2.0, v0.3.0, v1.0.0 Sep 16, 2024
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.

4 participants