-
Couldn't load subscription status.
- Fork 232
Strict typing for aiida.repository module #7049
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7049 +/- ##
==========================================
- Coverage 79.31% 79.30% -0.00%
==========================================
Files 566 566
Lines 43874 43877 +3
==========================================
- Hits 34794 34793 -1
- Misses 9080 9084 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| """ | ||
|
|
||
| @abc.abstractmethod | ||
| def maintain(self, dry_run: bool = False, live: bool = True, **kwargs) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was causing liskov principle violations in the subclasses. Ultimately, the "maintain" operation will be specialized for each different backend to probably doesn't make sense to have it here.
|
|
||
| __all__ = ('AbstractRepositoryBackend',) | ||
|
|
||
| InfoDictType = dict[str, Union[int, str, dict[str, int], dict[str, float]]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice to have a better type than this 😅
| with get_progress_reporter()(total=1) as progress: | ||
| callback = create_callback(progress) | ||
| container.pack_all_loose(compress=compress, callback=callback) | ||
| container.pack_all_loose(compress=compress_mode, callback=callback) # type: ignore[arg-type] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This ignore needs to be solved in the disk-objecstore package, I'll open a PR there.
| with get_progress_reporter()(total=1) as progress: | ||
| callback = create_callback(progress) | ||
| container.repack(callback=callback) | ||
| container.repack(callback=callback) # type: ignore[arg-type] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above, needs a fix in disk-objectstore repo
| instance = cls.__new__(cls) | ||
| instance.__init__(name, file_type, key, objects) # type: ignore[misc] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason why this was done in such a convoluted way??
3632e43 to
5809859
Compare
5809859 to
fbf9b7c
Compare
| f'Backend repository key format incompatible: {repository.key_format!r} != {key_format!r}' | ||
| ) | ||
| with get_progress_reporter()(desc='Archiving files: ', total=len(all_keys)) as progress: | ||
| for key, stream in repository.iter_object_streams(all_keys): # type: ignore[arg-type] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was the original motivation for this rabbithole :-D
Small changes in other modules were necessary pre-requisite or aftermath of better types in
aiida.repositorymodule.Some remaining type-ignores will need adjustment in the
disk-objectstorepackage, I'll open a PR there but it should not block this one.