-
Notifications
You must be signed in to change notification settings - Fork 7
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
chore: 📦 Update dependencies in poetry.lock and pyproject.toml #1642
Conversation
Review changes with SemanticDiff. Analyzed 1 of 3 files. Overall, the semantic diff is 50% smaller than the GitHub diff.
|
Reviewer's Guide by SourceryThis pull request updates type annotations in the test_converter.py file, specifically changing the type hints for NumPy arrays. The changes focus on improving type specificity and compatibility with newer typing conventions. Updated class diagram for test_converter.py type annotationsclassDiagram
class TestConverter {
- tmp_file_dict: Dict[str, Dict[str, NDArray[Shape["200"], Float]]]
+ tmp_file_dict: Dict[str, Dict[str, NDArray[Any]]]
- tmp_file_dict: Dict[str, Dict[str, NDArray[Shape["200"], Float]]]
+ tmp_file_dict: Dict[str, Dict[str, NDArray[np.float64]]]
}
note for TestConverter "Type annotations updated for better specificity and compatibility with newer typing conventions."
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Phylum OSS Supply Chain Risk Analysis - INCOMPLETEThe analysis contains 1 package(s) Phylum has not yet processed, |
|
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.
Hey @Anselmoo - I've reviewed your changes - here's some feedback:
Overall Comments:
- The PR title and description mention updating dependencies, but the actual changes appear to be related to type annotations in test files. Please update the PR title and description to accurately reflect the changes made.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟡 Testing: 2 issues found
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
@@ -403,7 +401,7 @@ def test_cmd_data_converter_nor_to_csv( | |||
|
|||
|
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.
suggestion (testing): Consider adding a test for the new, more flexible type annotation
The change from NDArray[Shape["200"], Float]
to NDArray[Any]
allows for more flexibility in the array shape and type. It would be beneficial to add a test case that uses an array with a different shape or type to ensure this change doesn't introduce any unexpected behavior.
@pytest.fixture(scope="function", autouse=True, name="tmp_file_dict")
def tmp_file_dict() -> Dict[str, Dict[str, NDArray[Any]]]:
"""Create temporary file with pickle data."""
return {
"test_data": {
"array_1d": np.random.rand(200),
"array_2d": np.random.rand(10, 20),
"array_3d": np.random.rand(5, 5, 5),
}
}
@@ -444,7 +442,7 @@ def tmp_file_nested_dict() -> Dict[str, Dict[str, Any]]: | |||
|
|||
@pytest.fixture(scope="function", autouse=True, name="tmp_pkl_gz_file") |
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.
suggestion (testing): Update test cases to cover new type annotation
The type annotation has changed from NDArray[Shape["200"], Float]
to NDArray[np.float64]
. Ensure that the existing test cases still cover all scenarios, and consider adding new test cases that specifically test the behavior with np.float64 arrays of various shapes.
tmp_path: Path, tmp_file_dict: Dict[str, Dict[str, NDArray[np.float64]]],
test_shapes: List[Tuple[int, ...]] = [(200,), (100, 2), (50, 2, 2)]
) -> Path:
"""Create temporary file with pickle data for various array shapes."""
for shape in test_shapes:
tmp_file_dict[f"test_array_{shape}"] = {
"data": np.random.rand(*shape).astype(np.float64)
}
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1642 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 45 45
Lines 4518 4516 -2
=========================================
- Hits 4518 4516 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
|
All PR-Submissions:
Pull Requests for the same
update/change?
New ✨✨ Feature-Submissions:
Changes to ⚙️ Core-Features:
us to include them?
Summary by Sourcery
Enhancements: