Some fixes to wrapper API
Merge request reports
Activity
1 # see https://docs.python.org/3/library/pkgutil.html 2 from pkgutil import extend_path 3 4 __path__ = extend_path(__path__, __name__) @tiago.pereira you only need this pkgutil hack for init files that are shared between packages.
Hey @amohammadi, thanks for the MR.
With respect to the transformers, I think the biggest difference between this MR and !185 (merged) is the way that you expanded the wrapped transformers.
I've created one transformer per element (preprocessor, extractor, algorithm) and a
wrap_bob_legacy
that wraps this with checkpoint and sample wrappers (i think this should be the default behavior). You explicitly created one class for each combination (TransformerPreprocessor, SamplePreprocessor,CheckpointPreprocessor...). Do you want to keep that? I think give a lot of options confuses more than helps. Besides, you can create this on the fly if needed.@tiago.pereira there is a lot of detail to take care of when you want to wrap legacy classes in SampleWrapper and CheckpointWrapper classes. For example:
- to know for preprocessors that
transform_extra_arguments=(("annotations", "annotations"),)
- To know for extractors that transform_extra_arguments, fit_extra_arguments changes depending on the extractor
- To know to pass the
model_path
to the extractor and not the CheckpointWrapper - And same things for algorithm.
It's not easy to figure these options out. So, it's best to provide them already IMO.
- to know for preprocessors that
Yeah, I know, but my functions take care of those cases.
For instance, Wrapping a preprocessor
wrap_preprocessor( PreprocesorTransformer(), features_dir="./", transform_extra_arguments=(("annotations", "annotations"),), ),
Wrapping an extractor
wrap_extractor( ExtractorTransformer(), features_dir="./" ),
Wrapping an algorithm
wrap_algorithm( AlgorithmTransformer(), features_dir="./", model_path="./xxx", fit_extra_arguments=(("subject", "y"),), ),
In case you want to benefit from some default path parameters, you can use directly
wrap_bob_legacy
function.wrap_bob_legacy( BobBioPreprocesor(), base_path, transform_extra_arguments=(("annotations", "annotations"),), ),
However, in case you know very well what you are doing, you can wrap your functions yourself with your stuff.
@tiago.pereira there is only one way to call these functions. If you leave it to the user, they will make errors. As I explained, when you wrap:
- A preprocessor, the
transform_extra_arguments=(("annotations", "annotations"),)
argument is mandatory. - A extractor, the
transform_extra_arguments = (("metadata", "metadata"),)
argument is mandatory if they accept metadata - A extractor, the
fit_extra_arguments = (("y", "subject"),)
argument is mandatory ifextractor.requires_training and extractor.split_training_data_by_client
and It should not be provided otherwise.
... Leaving this to the user is not a good idea.
Also, in the future, we may want to use these in different scenarios. So sometimes, I may just want a transformer, sometimes only a samplewrapper, and sometimes with checkpointing. So all these functions/classes will be used.
- A preprocessor, the
mentioned in merge request !185 (merged)