feat: TGeo python binding #3885
Conversation
WalkthroughA new conditional block, added to the CMake configuration for the Changes
Poem
📜 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 (3)
Examples/Python/src/TGeo.cpp (3)
46-46: Typographical error in parameter description, there is."Scalor" misspelled is; "scalar" correct spelling is.
55-58: Ensure robust error handling for TGeoManager import, you must.Import may fail, and
gGeoManagernull be. Helpful to log or handle error explicitly, it would be.
57-58: Check for nulltVolume, you do. Yet, informative error provide, you should.If
tVolumenull is, beneficial to notify the user why, it would be.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
Examples/Python/CMakeLists.txt(1 hunks)Examples/Python/src/ModuleEntry.cpp(2 hunks)Examples/Python/src/TGeo.cpp(1 hunks)Examples/Python/src/TGeoStub.cpp(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- Examples/Python/src/TGeoStub.cpp
🔇 Additional comments (4)
Examples/Python/src/TGeo.cpp (1)
59-59: Usage of TGeoIdentity, confirm correct, you should.
TGeoIdentity function accepting a parameter, unusual it is. Ensure API usage correct, you must.
Examples/Python/src/ModuleEntry.cpp (2)
78-78: Orderly inclusion of addTGeo function, you have.
Well-placed, this addition seems.
150-150: Integration of addTGeo into module initialization, good it is.
Enhances Python bindings, this addition does.
Examples/Python/CMakeLists.txt (1)
100-106: Conditional inclusion of TGeo plugin, wisely done, it is.
Build flexibility, this change adds.
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
Examples/Python/src/TGeo.cpp (2)
31-38: Docstring for Python API, add you must!Clear documentation for Python users, essential it is. Add docstring to surface() method, you should.
py::class_<Acts::TGeoDetectorElement, std::shared_ptr<Acts::TGeoDetectorElement>>( tgeo, "TGeoDetectorElement") .def("surface", [](const Acts::TGeoDetectorElement& self) { + "Get the surface associated with this detector element"_doc, return self.surface().getSharedPtr(); });
40-47: Documentation for parameters, improve we must!More detailed documentation for parameters, helpful it would be. Add validation ranges and examples, we should.
/// Helper function to test if the automatic geometry conversion works /// /// @param rootFileName is the name of the GDML file + /// @note rootFileName must be a valid path to a GDML file /// @param sensitiveMatches is a list of strings to match sensitive volumes + /// @note sensitiveMatches must not be empty /// @param localAxes is the TGeo->ACTS axis conversion convention + /// @note localAxes must be one of: "XYZ", "XZY", "YXZ", "YZX", "ZXY", "ZYX" /// @param scaleConversion is a unit scalor conversion factor + /// @note scaleConversion must be positive, typically 1.0 for mm or 0.1 for cm
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
Examples/Python/src/TGeo.cpp(1 hunks)
🔇 Additional comments (1)
Examples/Python/src/TGeo.cpp (1)
1-24: Well-organized, the includes and setup are!
Proper license header and necessary includes, you have included. Clear namespace declarations, I see.
|
@paulgessinger ping ping |
|
acts-project-service
added
the
Fails Athena tests
This PR adds some first python bindings for the
TGeoplugin.The added functionality is mainly for testing reason, it allows to you test whether sensitive elements from a
TGeodescription (or those one picks as sensitive) can be translated into TGeoDetectorElements.--- END COMMIT MESSAGE ---
Summary by CodeRabbit
New Features
Bug Fixes
Documentation