Move components out of experimental package#556
Conversation
stellasia
force-pushed
the
estelle/components-out-of-experimental
branch
from
080bf34 to
c80c8ed
Compare
|
|
||
|
|
||
| @runtime_checkable | ||
| class TaskProgressNotifierProtocol(Protocol): |
There was a problem hiding this comment.
Had to move these two classes here to avoid importing from experimental (+ avoid circular import issue)
| pass | ||
|
|
||
|
|
||
| class ComponentDefinitionError(Exception): |
There was a problem hiding this comment.
Replaces a PipelineDefinitionError to avoid importing from experimental (+ PipelineDefinitionError will probably be deprecated as we replace the pipeline)
| def __getattr__(name: str) -> Any: | ||
| warnings.warn( | ||
| f"Importing component {name} from the experimental namespace is deprecated and will be removed in version `2.0`. Use `from neo4j_graphrag.components... import ...` instead.", | ||
| DeprecationWarning, |
There was a problem hiding this comment.
I tested it and it seems that the deprecation warnings are not working as expected/documented.
For example, from neo4j_graphrag.experimental.components.entity_relation_extractor import LLMEntityRelationExtractor raises ModuleNotFoundError with no deprecation warning. This seems to needs a different mechanism to intercept submodule imports across the whole subtree? eg a sys.meta_path finder that redirects the whole subtree?
Worth adding a small unit test that imports moved path and asserts both that it resolves to the same object and that a DeprecationWarning is raised correctly.
| "Store", | ||
| "InMemoryStore", | ||
| "Component", | ||
| "DataModel", |
There was a problem hiding this comment.
should we also here re-export old imports + warn?
| - Import from the experimental namespace will be removed in 2.0 | ||
| - Update to use `from neo4j_graphrag.components... ` instead | ||
| - Improperly defined components now raise a `ComponentDefinitionError` instead of `PipelineDefinitionError` | ||
| - `TaskProgressNotifier` and `RunContext` have been moved to `neo4j_graphrag.components.base` |
There was a problem hiding this comment.
nit: TaskProgressNotifierProtocol instead of TaskProgressNotifier
2 participants
Description
Move all components out of the experimental namespace. In order not to import from experimental to the regular namespace, also had to move a couple of other classes:
Componentbase class that was inexperimental.pipeline.componentRunContextandTaskProgressNotifierProtocolwhich are part of theComponentdefinitionPipeineDefinitionErrortoComponentDefinitionErrorAll the rest is moving files around without changes and renaming imports.
Type of Change
Complexity
Complexity:
How Has This Been Tested?
Checklist
The following requirements should have been met (depending on the changes in the branch):