chore(ty): replace pyright with ty in RBMDS#56
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates RBMDataSet-related Python code and project configuration to use the ty type checker instead of Pyright, primarily by adjusting ty configuration and adding ty:ignore[...] suppressions where ty currently lacks inference/stub coverage.
Changes:
- Updated
pyproject.tomlto includeswvo/io/RBMDataSetintychecking (removed it fromexclude). - Replaced/added
ty:ignore[...]directives across RBMDataSet modules and scripts to address newtydiagnostics. - Cleaned up some Pyright-specific ignores and tightened some type annotations (e.g.,
Literal["mat", "pickle"]).
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
swvo/io/RBMDataSet/utils.py |
Adds ty suppressions around pandas datetime conversion and timezone handling. |
swvo/io/RBMDataSet/scripts/create_RBSP_line_data.py |
Adds ty suppressions in line-data creation logic and RBMDataSet attribute usage. |
swvo/io/RBMDataSet/interp_functions.py |
Adds Literal import and ty suppressions in interpolation functions. |
swvo/io/RBMDataSet/identify_orbits.py |
Removes Pyright-specific ignore comments for SciPy usage. |
swvo/io/RBMDataSet/bin_and_interpolate_to_model_grid.py |
Adds ty suppressions for typing issues in binning, plotting, and multiprocessing progress reporting. |
swvo/io/RBMDataSet/RBMNcDataSet.py |
Replaces broad type: ignore with ty-specific ignores for assignments and argument types. |
swvo/io/RBMDataSet/RBMDataSetManager.py |
Tightens preferred_extension to a Literal and adds a ty suppression for satellite typing. |
swvo/io/RBMDataSet/RBMDataSet.py |
Adds ty suppressions, removes some type: ignore, and tweaks inline ignore/formatting. |
pyproject.toml |
Enables ty checking for RBMDataSet by removing it from the excluded paths. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| joined_value = var_arr | ||
|
|
||
| loaded_var_arrs[key] = joined_value | ||
| loaded_var_arrs[key] = joined_value # ty:ignore[invalid-assignment] |
There was a problem hiding this comment.
loaded_var_arrs is annotated as dict[str, NDArray[np.number]], but joined_value may not always match that type, leading to ty:ignore[invalid-assignment]. Rather than suppressing, adjust the dict’s value type (e.g., include NDArray[np.object_] / NDArray[np.generic]) or refactor to keep numeric vs non-numeric entries separate.
|
|
||
| if target_type == TargetType.TargetPairs: | ||
| assert len(target_en) == len(target_al), "For TargetType.Pairs, the target vectors must have the same size!" | ||
| assert len(target_en) == len(target_al), "For TargetType.Pairs, the target vectors must have the same size!" # ty:ignore[invalid-argument-type] |
There was a problem hiding this comment.
Using assert for argument validation can be skipped when Python is run with optimizations (-O), which would remove this check. Prefer an explicit if ...: raise ValueError(...) here for user-facing input validation. Updating the types after normalization (e.g., cast target_en/target_al to list[float]) should also remove the need for the ty:ignore on this line.
| verbose: bool = True, | ||
| preferred_extension: str = "pickle", | ||
| preferred_extension: Literal["mat", "pickle"] = "pickle", | ||
| ) -> RBMDataSet | list[RBMDataSet]: |
There was a problem hiding this comment.
preferred_extension is now restricted to Literal["mat", "pickle"], but the overloads above still accept str, so type checkers will allow unsupported values. Either update the overload signatures to the same Literal[...] (and consider validating/raising on unexpected values) or revert the annotation to str with runtime validation.
| grid_X = grid_R[:, :, 0, 0] * np.cos(grid_P[:, :, 0, 0]) # ty:ignore[non-subscriptable] # ty:ignore[ignore-comment-unknown-rule, not-subscriptable] | ||
| grid_Y = grid_R[:, :, 0, 0] * np.sin(grid_P[:, :, 0, 0]) # ty:ignore[non-subscriptable] # ty:ignore[ignore-comment-unknown-rule, not-subscriptable] |
There was a problem hiding this comment.
The ty:ignore directives on these lines are duplicated and include ignore-comment-unknown-rule, which is not a valid diagnostic code. This can cause ty to reject the ignore comment and fail the type-check. Consolidate to a single ty:ignore[...] with only valid codes (or fix the underlying type issue) and remove the unknown rule name.
| grid_X = grid_R[:, :, 0, 0] * np.cos(grid_P[:, :, 0, 0]) # ty:ignore[non-subscriptable] # ty:ignore[ignore-comment-unknown-rule, not-subscriptable] | |
| grid_Y = grid_R[:, :, 0, 0] * np.sin(grid_P[:, :, 0, 0]) # ty:ignore[non-subscriptable] # ty:ignore[ignore-comment-unknown-rule, not-subscriptable] | |
| grid_X = grid_R[:, :, 0, 0] * np.cos(grid_P[:, :, 0, 0]) # ty:ignore[non-subscriptable] | |
| grid_Y = grid_R[:, :, 0, 0] * np.sin(grid_P[:, :, 0, 0]) # ty:ignore[non-subscriptable] |
No description provided.