-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[ENH] decouple registry
module from base classes
#6970
Labels
Comments
fkiraly
added
enhancement
Adding new functionality
module:base-framework
BaseObject, registry, base framework
labels
Aug 14, 2024
fkiraly
added a commit
that referenced
this issue
Aug 30, 2024
…rds for documentation of estimator types (#6998) Fixes #6970 by a redesign of the object scitype register that decouples the `registry` module from the rest of `sktime`, by removing all outside imports on module level (except from `sktime.base` and `utils`). As a side effect, this also adds one record class per object type, which can be used as a tagged metadata record and later as a basis or documenting the individual object types in `sktime`. The refactor proceeds as follows: * the base class register is replaced by data record classes similar to the `_tags` module * the imports of base classes are isolated in class methods of those records, `get_base_class`, which returns the base class corresponding to the scitype * exports of objects involving tags, in particular the classes, are replaced by functions that produce the object, further isolating the import to places where it is needed, e.g., `get_base_class_list` To make the changes deprecation safe, imports of coupled objects are intercepted, and replaced by calls on demand, i.e., whenever an external call carries out an import. Further, imports from outside `registry` but inside `sktime` (all from the test framework) are also replaced with the new functions. This lead to some dead functions and objects, which were removed, which further lead to unused imports, which also were removed.
github-project-automation
bot
moved this from In Progress
to Done
in 2024 May-Sep workstreams
Aug 30, 2024
benHeid
pushed a commit
to Z-Fran/sktime
that referenced
this issue
Oct 3, 2024
…rds for documentation of estimator types (sktime#6998) Fixes sktime#6970 by a redesign of the object scitype register that decouples the `registry` module from the rest of `sktime`, by removing all outside imports on module level (except from `sktime.base` and `utils`). As a side effect, this also adds one record class per object type, which can be used as a tagged metadata record and later as a basis or documenting the individual object types in `sktime`. The refactor proceeds as follows: * the base class register is replaced by data record classes similar to the `_tags` module * the imports of base classes are isolated in class methods of those records, `get_base_class`, which returns the base class corresponding to the scitype * exports of objects involving tags, in particular the classes, are replaced by functions that produce the object, further isolating the import to places where it is needed, e.g., `get_base_class_list` To make the changes deprecation safe, imports of coupled objects are intercepted, and replaced by calls on demand, i.e., whenever an external call carries out an import. Further, imports from outside `registry` but inside `sktime` (all from the test framework) are also replaced with the new functions. This lead to some dead functions and objects, which were removed, which further lead to unused imports, which also were removed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
The
registry
module is currently highly coupled to base classes - in the_base_classes
module, it imports all of them.This is a high degree of coupling, and is constantly leading to circular imports, e.g., if base classes want to access the registry, for instance for tags or scitype inference.
I think we should drastically reduce, optimally remove, the imports.
My suggestion would be to do this as follows:
registry._base_classes
objects, replace by use of tag system as much as possibleBASE_CLASS_LIST
by functions that export themsktime
internal footprint of the "list of classes" and similar objects exported byregistry
should be reduced as much as possible, and if possible eliminatedRelated issues:
Some user facing featuers which this would enable:
The text was updated successfully, but these errors were encountered: