tpaviot / pythonocc-core

Python package for 3D geometry CAD/BIM/CAM
GNU Lesser General Public License v3.0
1.39k stars 379 forks source link

Add hash eq and neq operations to TShape #1375

Closed JohannesVerherstraeten closed 2 weeks ago

JohannesVerherstraeten commented 1 month ago

Fixes the issue where the __hash__, __eq__ and __neq__ operators of TopoDS_TShape are not implemented, giving unexpected results like:

from OCC.Core.BRepPrimAPI import BRepPrimAPI_MakeBox

shape_1 = BRepPrimAPI_MakeBox(10, 20, 30).Shape()

shape_1.TShape() == shape_1.TShape()    # Expected: True, but is False
hash(shape_1.TShape()) == hash(shape_1.TShape())    # Expected: True, but is False

This fix allows properly comparing TopoDS_TShapes and using them in sets/dictionaries.

Summary by Sourcery

Implement hash, equality, and inequality operators for TopoDS_TShape to fix comparison issues and enable usage in sets and dictionaries, and add corresponding tests.

New Features:

Tests:

sourcery-ai[bot] commented 1 month ago

Reviewer's Guide by Sourcery

This pull request implements hash, equality, and inequality operations for the TShape class in the TopoDS module. It addresses an issue where these operations were not properly implemented, leading to unexpected behavior when comparing TShape objects or using them in sets/dictionaries.

Class diagram for updated TopoDS_TShape operations

classDiagram
    class TopoDS_TShape {
        +__hash__() size_t
        +__eq__(self, right) bool
        +__ne__(self, right) bool
    }
    note for TopoDS_TShape "Added hash, eq, and neq operations"

File-Level Changes

Change Details Files
Implement hash, eq, and ne methods for TopoDS_TShape
  • Add ne_wrapper method to handle inequality comparisons
  • Add eq_wrapper method to handle equality comparisons
  • Implement hash method using opencascade::hash
  • Add Python code to properly handle eq and ne operations
src/SWIG_files/wrapper/TopoDS.i
Add unit tests for new TShape operations
  • Create test_tshape_hash_operator to verify hash functionality
  • Create test_tshape_eq_operator to verify equality comparisons
  • Create test_tshape_neq_operator to verify inequality comparisons
test/test_core_wrapper_features.py

Tips and commands #### Interacting with Sourcery - **Trigger a new review:** Comment `@sourcery-ai review` on the pull request. - **Continue discussions:** Reply directly to Sourcery's review comments. - **Generate a GitHub issue from a review comment:** Ask Sourcery to create an issue from a review comment by replying to it. - **Generate a pull request title:** Write `@sourcery-ai` anywhere in the pull request title to generate a title at any time. - **Generate a pull request summary:** Write `@sourcery-ai summary` anywhere in the pull request body to generate a PR summary at any time. You can also use this command to specify where the summary should be inserted. #### Customizing Your Experience Access your [dashboard](https://app.sourcery.ai) to: - Enable or disable review features such as the Sourcery-generated pull request summary, the reviewer's guide, and others. - Change the review language. - Add, remove or edit custom review instructions. - Adjust other review settings. #### Getting Help - [Contact our support team](mailto:support@sourcery.ai) for questions or feedback. - Visit our [documentation](https://docs.sourcery.ai) for detailed guides and information. - Keep in touch with the Sourcery team by following us on [X/Twitter](https://x.com/SourceryAI), [LinkedIn](https://www.linkedin.com/company/sourcery-ai/) or [GitHub](https://github.com/sourcery-ai).
tpaviot commented 2 weeks ago

Thank you @JohannesVerherstraeten for this contribution. It is strange the _neq__ or eq operators are not handled by default for the TopoDS_TShape class, all c++ classes that implement the != operator should benefit from this addition.

tpaviot commented 2 weeks ago

superseded by #1382

JohannesVerherstraeten commented 2 weeks ago

Thanks tpaviot, I'll close this PR

tpaviot commented 2 weeks ago

thank you @JohannesVerherstraeten feel free to contribute swig stuff whenever you want, I'll review changes as fast as I can