CU-8694pey4u: extract cdb load to cls method #446
Conversation
…or model pack loading
|
Task linked: CU-8694pey4u extract cdb load to cls method |
medcat/cat.py
Outdated
| json_path = model_pack_path if has_jsons else None | ||
| logger.info('Loading model pack with %s', 'JSON format' if json_path else 'dill format') | ||
| cdb = CDB.load(cdb_path, json_path) | ||
| cdb = cls.load_cdb(model_pack_path) |
There was a problem hiding this comment.
I don't really know why the local imports are here (or above in the method at least). My best guess is that at some point this helped avoid circular imports while at the same time provided a guarantee that the classes would be available during model pack loading. But currently, this shouldn't really be an issue since all 4 are imported at the top of the module anyway so there's no reason they wouldn't be available during operation.
So I don't see why we wouldn't just remove the local imports in here.
With that said, that would probably fall outside the scope of this PR.
So I think the easiest solution to the linting issue would be to type-annotate here, e.g cdb: CDB = cls.load_cdb(model_pack_path).
There was a problem hiding this comment.
done - yeah on above local imports. Agreed prob due to circular imports somewhere... and yes outside of scope of this PR
medcat/cat.py
Outdated
| meta_cats.append(MetaCAT.load(save_dir_path=meta_path, | ||
| config_dict=meta_cat_config_dict)) | ||
| if load_meta_models: | ||
| meta_cats = cls.load_meta_cats(model_pack_path) |
There was a problem hiding this comment.
Same overall comment as above.
So I think the easiest solution to the linting issue would be to type-annotate here, e.g meta_cats: List[MetaCAT] = cls.load_meta_cats(model_pack_path).
tomolopolis
force-pushed
the
load-cdb-cls-method
branch
2 times, most recently
from
661ab13
to
e1a34ab
Compare
tomolopolis
force-pushed
the
load-cdb-cls-method
branch
from
e1a34ab
to
bc78a53
Compare
to be used in trainer for model pack loading