Fix: pyright error: Type "floating[Any]" is not assignable to return type "ndarray[Unknown, Unknown]" #765
Conversation
…type "ndarray[Unknown, Unknown]"
CodSpeed Performance ReportMerging #765 will not alter performanceComparing Summary
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## devel #765 +/- ##
==========================================
- Coverage 85.15% 85.02% -0.14%
==========================================
Files 81 81
Lines 7526 7517 -9
==========================================
- Hits 6409 6391 -18
- Misses 1117 1126 +9 ☔ View full report in Codecov by Sentry. |
📝 Walkthrough📝 WalkthroughWalkthroughThe changes in this pull request involve updates to three files: Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PreCommit
participant ErrorsBase
User->>PreCommit: Trigger pre-commit hooks
PreCommit->>ErrorsBase: Lint and format code
ErrorsBase-->>PreCommit: Return formatted code
PreCommit-->>User: Complete pre-commit checks
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
dpdata/stat.py (1)
78-93: LGTM: Property type annotations are consistentThe property return types are correctly updated to match the underlying mae/rmse function signatures.
Consider adding type hints to the error arrays in
e_errorsandf_errorsabstract properties to complete the typing chain:@property @abstractmethod -def e_errors(self) -> np.ndarray: +def e_errors(self) -> np.ndarray[Any, np.dtype[np.floating[Any]]]: """Energy errors.""" @property @abstractmethod -def f_errors(self) -> np.ndarray: +def f_errors(self) -> np.ndarray[Any, np.dtype[np.floating[Any]]]: """Force errors."""
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
.pre-commit-config.yaml(1 hunks)dpdata/stat.py(4 hunks)
✅ Files skipped from review due to trivial changes (1)
- .pre-commit-config.yaml
🔇 Additional comments (3)
dpdata/stat.py (3)
5-5: LGTM: Required import for type annotations
The addition of typing.Any import is necessary for the updated type annotations.
Line range hint 28-38: LGTM: Consistent type annotation update
The change to np.floating[Any] is consistent with the mae function and correctly represents the return type of the numpy operation chain.
Line range hint 12-22: LGTM: Type annotation matches numpy's behavior
The change from np.float64 to np.floating[Any] is correct as np.mean() can return different floating-point types depending on the input array's dtype.
✅ Verification successful
Let me verify the implementation of the mae function to ensure the return type annotation is correct.
Let me try a different pattern to get the function implementation.
LGTM: Type annotation change is correct
The implementation of mae function uses np.mean(np.abs(errors)) which returns a numpy floating-point type. The change from np.float64 to np.floating[Any] is more accurate as the return type depends on the input array's dtype. This is consistent across the codebase where other functions like rmse and error properties also use the same return type annotation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify numpy's mean return type behavior
ast-grep --pattern 'np.mean($$$)' | rg -A 2 'mean'
Length of output: 259
Script:
#!/bin/bash
# Get the complete implementation of mae function
ast-grep --pattern 'def mae($_) {
$$$
}'
Length of output: 44
Script:
#!/bin/bash
# Get the complete implementation of mae function
cat dpdata/stat.py
Length of output: 4633
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
dpdata/system.py (1)
1052-1052: Good addition for type safety, but error message could be more descriptive.The assertion helps catch cases where
picked_atom_idxis a scalar boolean instead of a boolean array, which would cause type errors. This aligns with fixing the Pyright type error mentioned in the PR description.Consider making the assertion message more descriptive to help developers understand the issue:
- assert not isinstance(picked_atom_idx, np.bool) + assert not isinstance(picked_atom_idx, np.bool), "picked_atom_idx must be a boolean array, not a scalar boolean. This usually happens when all atoms are selected/deselected in the boolean operation."
Signed-off-by: Jinzhe Zeng <[email protected]>
Summary by CodeRabbit
ruff-pre-commitrepository for improved linting and formatting of Python code.remove_atom_namesmethod by adding an assertion to prevent type-related issues.