Fix some type errors #3440
Merged
Fix some type errors #3440
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is needed to correctly match types in functions that take unions as arguments and pass those to functions annotated with VariableLikeType.
The previous suppression ignored the fact that some packages modules are implemented in their __init__ files, that is, the __init__ files are not simply collections of imports.
|
Note that I got lazy towards the end and started using python 3.10 features. So this PR will only work once we drop 3.9 in CI. |
# Conflicts: # src/scipp/compat/wrapping.py # src/scipp/core/_sizes.py # src/scipp/core/assignments.py # src/scipp/core/concepts.py # src/scipp/core/variable.py # src/scipp/format/_parse.py # src/scipp/format/formatter.py # src/scipp/scipy/integrate/__init__.py # src/scipp/scipy/interpolate/__init__.py # src/scipp/scipy/ndimage/__init__.py # src/scipp/scipy/optimize/__init__.py # src/scipp/scipy/signal/__init__.py # src/scipp/units/__init__.py # tools/stubgen/__init__.py # tools/stubgen/transformer.py
YooSunYoung
reviewed
May 14, 2024
YooSunYoung
reviewed
May 14, 2024
src/scipp/core/_sizes.py
Outdated
|
|
||
|
|
||
| def _parse_dims_shape_sizes( | ||
| dims: list[str] | tuple[str, ...] | None = None, | ||
| shape: Sequence[int] | None = None, | ||
| sizes: dict[str, int] | None = None, | ||
| ): | ||
| ) -> Any: |
There was a problem hiding this comment.
Good question. I think it was just a quick and easy way to make Mypy shut up because this is an internal function. Changed it to be a bit more accurate.
YooSunYoung
approved these changes
May 15, 2024
src/scipp/core/_cpp_wrapper_util.py
Outdated
Comment on lines
10
to
11
| if TYPE_CHECKING: | ||
| from .data_group import DataGroup |
There was a problem hiding this comment.
Why is it needed here? it's already imported in LN8
src/scipp/core/operations.py
Outdated
| @@ -305,7 +322,7 @@ def merge(lhs: Dataset, rhs: Dataset) -> Dataset: ... | |||
| def merge(lhs: DataGroup, rhs: DataGroup) -> DataGroup: ... | |||
|
|
|||
|
|
|||
| def merge(lhs, rhs): | |||
| def merge(lhs: Dataset | DataGroup, rhs: Dataset | DataGroup) -> Dataset | DataGroup: | |||
Comment on lines
+31
to
+33
| def __init__(self, keys: list[str], var: Variable) -> None: | ||
| self._var = var | ||
| self._keys = keys |
There was a problem hiding this comment.
Does the _keys need to have the reference to the original keys?
Suggested change
| def __init__(self, keys: list[str], var: Variable) -> None: | |
| self._var = var | |
| self._keys = keys | |
| def __init__(self, keys: Sequence[str], var: Variable) -> None: | |
| self._var = var | |
| self._keys = tuple(keys) |
There was a problem hiding this comment.
It doesn't. This was to avoid making a copy because that is not needed here. This class should never be instantiated by anyone outside of this module.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes some (a small fraction) type errors. Most of this is internal code. But making Mypy happy with it pointed out some problems in lower level annotations. So in particular the fixes in
cpp_classes.pyi(by way of changes in C++ and the stubgen) should improve the situation for users.There are many problems remaining. Even in the stub file itself. But those in particular need either a more sophisticated stubgen or an alternative solution. I didn't want to invest more time in it now.
Note that I refactored a few modules to help with type checks by making function interfaces clearer.