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

[ENH] decouple registry module from base classes #6970

Closed
fkiraly opened this issue Aug 14, 2024 · 0 comments · Fixed by #6998
Closed

[ENH] decouple registry module from base classes #6970

fkiraly opened this issue Aug 14, 2024 · 0 comments · Fixed by #6998
Assignees
Labels
enhancement Adding new functionality module:base-framework BaseObject, registry, base framework

Comments

@fkiraly
Copy link
Collaborator

fkiraly commented Aug 14, 2024

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:

  • in uses of the registry._base_classes objects, replace by use of tag system as much as possible
  • move the base class registry to a dataclass-like structure, which can isolate the class imports
  • replacement of class containing objects such as BASE_CLASS_LIST by functions that export them
    • removal of the actual class containing objects will need to go through deprecation, as they are publicly exported.
    • the sktime internal footprint of the "list of classes" and similar objects exported by registry should be reduced as much as possible, and if possible eliminated

Related issues:

Some user facing featuers which this would enable:

@fkiraly fkiraly added enhancement Adding new functionality module:base-framework BaseObject, registry, base framework labels Aug 14, 2024
@fkiraly fkiraly self-assigned this Aug 14, 2024
@fkiraly fkiraly moved this to In Progress in 2024 May-Sep workstreams 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 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
enhancement Adding new functionality module:base-framework BaseObject, registry, base framework
Projects
Status: Done
1 participant