Restore Codacy#146
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (15)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (7)
📜 Recent review details⏰ Context from checks skipped due to timeout. (15)
🔇 Additional comments (9)
📝 WalkthroughWalkthroughThe pull request revises the CMake/Python build flow, pins CI actions, updates Docker and analysis configuration, adjusts several C++ implementations, reformats Python examples and tests, and refreshes repository documentation. ChangesRepository maintenance
Estimated code review effort: 4 (Complex) | ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | -7 |
| Duplication | -3 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Includes/Core/CUDA/CUDAPointHashGridSearcher2.hpp (1)
232-236: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winInitialize
m_dummyinCUDAPointHashGridSearcher2
m_dummystill has no default initializer, and the constructors, move ops, andSet()never assign it. Useuint1 m_dummy{};to avoid an indeterminate member.🔧 Suggested fix
float m_gridSpacing = 1.0f; - uint1 m_dummy; + uint1 m_dummy{}; uint2 m_resolution = make_uint2(1, 1);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Includes/Core/CUDA/CUDAPointHashGridSearcher2.hpp` around lines 232 - 236, Initialize the m_dummy member in CUDAPointHashGridSearcher2 with value initialization, changing its declaration to use uint1 m_dummy{}; so it is never indeterminate.
🧹 Nitpick comments (4)
Sources/Core/Geometry/Cylinder3.cpp (1)
211-218: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winSame duplicate-sqrt pattern remains in
ClosestIntersectionLocal.
IntersectsLocalabove was just refactored to precomputesqrtDiscriminantonce instead of callingstd::sqrt(B * B - A * C)twice. This function still has the original duplicated computation fort1/t2. Consider applying the same simplification here for consistency and to avoid the redundantsqrtcall.♻️ Suggested consistency fix
- double t1 = (-B + std::sqrt(B * B - A * C)) / A; - double t2 = (-B - std::sqrt(B * B - A * C)) / A; + const double sqrtDiscriminant = std::sqrt(B * B - A * C); + double t1 = (-B + sqrtDiscriminant) / A; + double t2 = (-B - sqrtDiscriminant) / A; double tCylinder = t2;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/Core/Geometry/Cylinder3.cpp` around lines 211 - 218, In ClosestIntersectionLocal, compute the discriminant square root once in a local variable and reuse it when calculating t1 and t2, matching the existing IntersectsLocal refactor and eliminating the duplicate std::sqrt call.setup.py (1)
54-57: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer iterable unpacking over list concatenation (RUF005).
Static analysis flags both
self.spawn(...)calls for list-concatenation style. Since this PR's goal is restoring Codacy/lint compliance, worth fixing directly.♻️ Proposed fix
- self.spawn( - ["cmake", "-S", ext.sourcedir, "-B", self.build_temp] + cmake_args - ) - self.spawn(["cmake", "--build", self.build_temp] + build_args) + self.spawn( + ["cmake", "-S", ext.sourcedir, "-B", self.build_temp, *cmake_args] + ) + self.spawn(["cmake", "--build", self.build_temp, *build_args])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@setup.py` around lines 54 - 57, Replace list concatenation in both self.spawn calls within build_ext.build_extension with iterable unpacking, using [“cmake”, …, *cmake_args] and [“cmake”, …, *build_args] while preserving the existing command ordering and arguments.Source: Linters/SAST tools
.github/workflows/windows.yml (1)
40-40: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSHA pin verified correct; consider
v3for a future bump.Confirmed the commit SHA "Address issue
#125releasing the node20 action as major version change" corresponds to thev2tag ofmicrosoft/setup-msbuild. The action's current marketplace guidance already recommends@v3, so this pin is valid but slightly behind the latest major version.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/windows.yml at line 40, Update the microsoft/setup-msbuild action reference in the Windows workflow from the pinned v2 commit to a verified commit corresponding to the current v3 release, and update the inline version comment accordingly.README.md (1)
155-166: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win"The class is licensed under..." reads oddly for a project README.
This phrasing appears to be leftover boilerplate wording (referring to "the class") rather than the project. Since this section is being reformatted anyway, worth fixing the wording for clarity.
✏️ Proposed fix
-The class is licensed under the [MIT License](http://opensource.org/licenses/MIT): +This project is licensed under the [MIT License](http://opensource.org/licenses/MIT):🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 155 - 166, Replace the boilerplate phrase “The class is licensed under...” with clear project-level wording, such as stating that the project is licensed under the MIT License, while preserving the existing license link and attribution list.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Examples/PythonExamples/apic_example_01.py`:
- Line 1: The wildcard import in apic_example_01.py triggers Ruff F403; replace
it with explicit imports for the symbols used, or add a targeted
suppression/configuration consistent with the examples directory. Ensure Ruff no
longer reports this file while preserving its required functionality.
In `@Examples/PythonExamples/apic_example_02.py`:
- Line 1: The wildcard import in apic_example_02.py triggers Ruff F403. Replace
it with explicit imports for the symbols used in the example, or add a targeted
F403 suppression consistent with the other Python examples.
In `@Examples/PythonExamples/apic_example_03.py`:
- Line 1: The wildcard import in apic_example_03.py triggers Ruff F403; replace
it with explicit imports for the symbols used in the example, or add a targeted
F403 suppression consistent with the examples directory’s lint configuration.
In `@Examples/PythonExamples/collider_emitter_anim_example.py`:
- Line 1: The wildcard import in collider_emitter_anim_example.py triggers Ruff
F403; replace it with explicit symbols imported from pyCubbyFlow, or add a
targeted F403 suppression consistent with the examples directory’s lint
configuration.
In `@Examples/PythonExamples/points_to_implicit.py`:
- Line 1: Fix the Ruff F403 violation in points_to_implicit.py by replacing the
wildcard import with explicit pyCubbyFlow imports used by the example;
alternatively, add a narrowly scoped suppression or apply consistent Ruff
configuration for the examples directory.
In `@Examples/PythonExamples/smoke_example_01.py`:
- Line 1: The wildcard import in smoke_example_01.py triggers Ruff F403; replace
it with explicit pyCubbyFlow imports, or add a targeted F403
suppression/configuration consistent with the examples directory.
---
Outside diff comments:
In `@Includes/Core/CUDA/CUDAPointHashGridSearcher2.hpp`:
- Around line 232-236: Initialize the m_dummy member in
CUDAPointHashGridSearcher2 with value initialization, changing its declaration
to use uint1 m_dummy{}; so it is never indeterminate.
---
Nitpick comments:
In @.github/workflows/windows.yml:
- Line 40: Update the microsoft/setup-msbuild action reference in the Windows
workflow from the pinned v2 commit to a verified commit corresponding to the
current v3 release, and update the inline version comment accordingly.
In `@README.md`:
- Around line 155-166: Replace the boilerplate phrase “The class is licensed
under...” with clear project-level wording, such as stating that the project is
licensed under the MIT License, while preserving the existing license link and
attribution list.
In `@setup.py`:
- Around line 54-57: Replace list concatenation in both self.spawn calls within
build_ext.build_extension with iterable unpacking, using [“cmake”, …,
*cmake_args] and [“cmake”, …, *build_args] while preserving the existing command
ordering and arguments.
In `@Sources/Core/Geometry/Cylinder3.cpp`:
- Around line 211-218: In ClosestIntersectionLocal, compute the discriminant
square root once in a local variable and reuse it when calculating t1 and t2,
matching the existing IntersectsLocal refactor and eliminating the duplicate
std::sqrt call.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 36ebd5a6-2cf7-4045-935b-cd38e32b29b6
📒 Files selected for processing (30)
.codacy.yml.dockerignore.github/ISSUE_TEMPLATE/bug_report.md.github/workflows/docs.yml.github/workflows/ubuntu-codecov.yml.github/workflows/windows-cuda.yml.github/workflows/windows.ymlDockerfileExamples/PythonExamples/apic_example_01.pyExamples/PythonExamples/apic_example_02.pyExamples/PythonExamples/apic_example_03.pyExamples/PythonExamples/collider_emitter_anim_example.pyExamples/PythonExamples/points_to_implicit.pyExamples/PythonExamples/smoke_example_01.pyExamples/SmokeSim/main.cppIncludes/Core/CUDA/CUDAPointHashGridSearcher2.hppIncludes/Core/CUDA/CUDAPointHashGridSearcher3.hppIncludes/Core/Math/SVD-Impl.hppREADME.mdSources/Core/Geometry/Cylinder3.cppTests/PythonTests/pytest_utils.pyTests/PythonTests/test_animation.pyTests/PythonTests/test_cell_centered_scalar_grid.pyTests/PythonTests/test_marching_cubes.pyTests/PythonTests/test_physics_animation.pyTests/PythonTests/test_sphere.pyTests/PythonTests/test_vertex_centered_scalar_grid.pyTests/UnitTests/Vector2Tests.cppTests/UnitTests/Vector3Tests.cppsetup.py
💤 Files with no reviewable changes (2)
- Tests/PythonTests/test_marching_cubes.py
- Tests/PythonTests/pytest_utils.py
📜 Review details
⏰ Context from checks skipped due to timeout. (16)
- GitHub Check: CodeRabbit / Review
- GitHub Check: 🐧 CUDA Build - Ubuntu 24.04 + gcc-12 + CUDA 12.6.3
- GitHub Check: 🍎 Build - macOS 15.7.4 + Xcode 16.4
- GitHub Check: 🍎 Build - macOS 26.3 + Xcode 26.3
- GitHub Check: 🧪 Code Coverage - Codecov (Ubuntu 24.04 + gcc-14, ubuntu-24.04, gcc, 14)
- GitHub Check: 🪟 Build - Windows Server 2025 + Visual Studio 2026
- GitHub Check: 🪟 Build - Windows Server 2022 + Visual Studio 2022
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: 🪟 CUDA Build - Windows Server 2025 + Visual Studio 2026 + CUDA 13.2.0 (Release)
- GitHub Check: 🪟 CUDA Build - Windows Server 2022 + Visual Studio 2022 + CUDA 12.6.3 (Release)
- GitHub Check: 🐧 Build - Ubuntu 24.04 + gcc-12
- GitHub Check: 🐧 Build - Ubuntu 24.04 + clang-18
- GitHub Check: 🐧 Build - Ubuntu 24.04 + clang-17
- GitHub Check: 🐧 Build - Ubuntu 24.04 + clang-16
- GitHub Check: 🐧 Build - Ubuntu 24.04 + gcc-13
- GitHub Check: 🐧 Build - Ubuntu 24.04 + gcc-14
🧰 Additional context used
🪛 LanguageTool
.github/ISSUE_TEMPLATE/bug_report.md
[grammar] ~38-~38: Ensure spelling is correct
Context: ...following information):** - Device: [e.g. iPhone6] - OS: [e.g. iOS8.1] - Browser [e.g. stock ...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 Ruff (0.15.20)
Examples/PythonExamples/points_to_implicit.py
[error] 1-1: from pyCubbyFlow import * used; unable to detect undefined names
(F403)
Examples/PythonExamples/apic_example_01.py
[error] 1-1: from pyCubbyFlow import * used; unable to detect undefined names
(F403)
Examples/PythonExamples/apic_example_02.py
[error] 1-1: from pyCubbyFlow import * used; unable to detect undefined names
(F403)
Examples/PythonExamples/collider_emitter_anim_example.py
[error] 1-1: from pyCubbyFlow import * used; unable to detect undefined names
(F403)
Examples/PythonExamples/apic_example_03.py
[error] 1-1: from pyCubbyFlow import * used; unable to detect undefined names
(F403)
Examples/PythonExamples/smoke_example_01.py
[error] 1-1: from pyCubbyFlow import * used; unable to detect undefined names
(F403)
setup.py
[warning] 55-55: Consider iterable unpacking instead of concatenation
Replace with iterable unpacking
(RUF005)
[warning] 57-57: Consider ["cmake", "--build", self.build_temp, *build_args] instead of concatenation
Replace with ["cmake", "--build", self.build_temp, *build_args]
(RUF005)
🔇 Additional comments (32)
Examples/PythonExamples/apic_example_01.py (1)
17-17: LGTM!Also applies to: 28-31, 48-56
Examples/PythonExamples/apic_example_02.py (1)
21-21: LGTM!Also applies to: 32-35, 52-60
Examples/PythonExamples/apic_example_03.py (1)
16-52: LGTM!Examples/PythonExamples/collider_emitter_anim_example.py (1)
28-29: LGTM!Also applies to: 40-43, 63-86
Examples/PythonExamples/points_to_implicit.py (1)
12-12: LGTM!Examples/PythonExamples/smoke_example_01.py (1)
31-39: LGTM!Also applies to: 49-67, 70-70
Tests/PythonTests/test_animation.py (1)
6-6: LGTM!Tests/PythonTests/test_cell_centered_scalar_grid.py (1)
12-47: LGTM!Also applies to: 65-65, 81-88, 114-138, 174-196, 205-217, 237-237, 251-251, 260-260, 289-313
Tests/PythonTests/test_physics_animation.py (1)
7-7: LGTM!Tests/PythonTests/test_sphere.py (1)
6-6: LGTM!Tests/PythonTests/test_vertex_centered_scalar_grid.py (1)
12-47: LGTM!Also applies to: 65-65, 81-88, 114-138, 174-196, 205-217, 237-237, 251-251, 260-260, 289-313
Tests/UnitTests/Vector2Tests.cpp (1)
171-173: LGTM!Also applies to: 255-255
Tests/UnitTests/Vector3Tests.cpp (1)
188-190: LGTM!Also applies to: 276-276
Examples/SmokeSim/main.cpp (1)
36-37: LGTM!Also applies to: 78-96, 552-552
Includes/Core/CUDA/CUDAPointHashGridSearcher2.hpp (2)
56-58: HashUtils members correctly given default initializers.Good fix for uninitialized-member warnings.
276-276: LGTM!Includes/Core/CUDA/CUDAPointHashGridSearcher3.hpp (1)
56-57: LGTM!Also applies to: 275-275
Includes/Core/Math/SVD-Impl.hpp (1)
59-59: LGTM!Also applies to: 447-447, 816-816
Sources/Core/Geometry/Cylinder3.cpp (2)
119-126: LGTM!Nice simplification: precomputing
sqrtDiscriminantavoids the duplicatestd::sqrtcall and only computes the alternate root when needed.
308-308: LGTM!Dockerfile (2)
7-11: 🩺 Stability & Availability | ⚡ Quick winPinned apt package versions may become unresolvable over time.
build-essential=12.8ubuntu1.1,python3-dev=3.8.2-0ubuntu2, andpython3-pip=20.0.2-5ubuntu1.11pin exact patch versions from the Ubuntu 20.04 archive. Canonical's standard archive typically retains only the latest patched build of each package; once a security update supersedes these exact versions,apt-get install pkg=versionwill fail with "Version not found", breaking the image build unintentionally.Consider pinning only major/minor versions (or none) for apt packages, or use
snapshot.ubuntu.comif exact reproducibility is required.
22-24: LGTM!setup.py (2)
12-53: LGTM!
60-68: LGTM!.github/ISSUE_TEMPLATE/bug_report.md (1)
1-46: LGTM!(The LanguageTool "iPhone6" spelling hint is a false positive — it's GitHub's standard bug-report placeholder text, unrelated to this diff.)
README.md (2)
3-8: LGTM!
137-143: LGTM!.codacy.yml (1)
1-5: LGTM!.dockerignore (1)
1-2: LGTM!.github/workflows/docs.yml (1)
26-26: LGTM!.github/workflows/ubuntu-codecov.yml (1)
57-57: LGTM!.github/workflows/windows-cuda.yml (1)
72-72: LGTM!
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #146 +/- ##
==========================================
- Coverage 84.37% 84.36% -0.01%
==========================================
Files 460 460
Lines 32472 32468 -4
Branches 2796 2799 +3
==========================================
- Hits 27397 27393 -4
Misses 5075 5075 🚀 New features to boost your workflow:
|
1 participant
This revision includes:
Summary by CodeRabbit