824 refactor implement combinedsteelcrosssection with the possibility of cross sections with multiple steel grades #826
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 734-refactor-steel-profiles #826 +/- ##
=============================================================
Coverage 100.00% 100.00%
=============================================================
Files 397 399 +2
Lines 12249 12298 +49
=============================================================
+ Hits 12249 12298 +49 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
This PR implements a CombinedSteelCrossSection class to enable steel cross-sections composed of multiple components with potentially different steel grades. The implementation follows a builder-composite pattern using immutable dataclasses with a fluent API.
Key changes:
- Introduced
SteelCrossSectionProtocolto define the common interface for steel cross-sections - Added position/orientation fields (
x_offset,y_offset,rotation_angle) toSteelCrossSectionwith a_transformmethod for internal use - Created
CombinedSteelCrossSectionwith a builder pattern API viaadd_steel_cross_sectionmethod - Updated test files to use raw strings (r-prefix) for regex patterns in pytest assertions
Reviewed Changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| blueprints/structural_sections/steel/steel_cross_section.py | Adds Protocol definition, position/orientation fields, and internal _transform method to support combined sections |
| blueprints/structural_sections/steel/combined_steel_cross_section.py | New class implementing the builder-composite pattern for combining multiple steel cross-sections |
| blueprints/structural_sections/steel/init.py | Exports new classes for public API |
| tests/structural_sections/steel/test_steel_cross_section.py | Adds tests for new position/orientation fields and _transform method |
| tests/structural_sections/steel/test_combined_steel_cross_section.py | Comprehensive test suite for CombinedSteelCrossSection functionality |
| tests/structural_sections/test_polygon_builder.py | Updates regex patterns to use raw strings |
| tests/structural_sections/steel/steel_profile_sections/test_strip_profile.py | Updates regex patterns to use raw strings |
| tests/structural_sections/steel/steel_profile_sections/test_rhs_profile.py | Updates regex patterns to use raw strings |
| tests/structural_sections/steel/steel_profile_sections/test_i_profile.py | Updates regex patterns to use raw strings |
| tests/structural_sections/steel/steel_profile_sections/test_chs_profile.py | Updates regex patterns to use raw strings |
| tests/structural_sections/geometric_cross_sections/test_cross_section_tube.py | Updates regex patterns to use raw strings |
| tests/structural_sections/geometric_cross_sections/test_cross_section_triangle.py | Updates regex patterns to use raw strings |
| tests/structural_sections/geometric_cross_sections/test_cross_section_rectangle.py | Updates regex patterns to use raw strings |
| tests/structural_sections/geometric_cross_sections/test_cross_section_hexagon.py | Updates regex patterns to use raw strings |
| tests/structural_sections/geometric_cross_sections/test_cross_section_cornered.py | Updates regex patterns to use raw strings |
| tests/structural_sections/geometric_cross_sections/test_cross_section_circle.py | Updates regex patterns to use raw strings |
| tests/structural_sections/geometric_cross_sections/test_cross_section_annular_sector.py | Updates regex patterns to use raw strings |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| y_offset : MM, optional | ||
| The y-coordinate offset of the cross-section's centroid [mm]. Default is 0.0. | ||
| rotation_angle : DEG, optional | ||
| The rotation angle of the cross-section in degrees (counter-clockwise). Default is 0.0 |
There was a problem hiding this comment.
Missing period at the end of the sentence. The description should end with a period for consistency with other parameter descriptions in the docstring.
| The rotation angle of the cross-section in degrees (counter-clockwise). Default is 0.0 | |
| The rotation angle of the cross-section in degrees (counter-clockwise). Default is 0.0. |
| @@ -0,0 +1,127 @@ | |||
| """Module containing the class definition for a combined steel cross-section, allowing for cross-sections with multiple steel materials.""" | |||
There was a problem hiding this comment.
[nitpick] The module docstring mentions "multiple steel materials" but the implementation allows combining multiple steel cross-sections that may differ in geometry, material, or both. Consider using more precise wording like "allowing for combined cross-sections with multiple steel components" or "enabling cross-sections composed of multiple steel parts with potentially different materials and geometries".
| """Module containing the class definition for a combined steel cross-section, allowing for cross-sections with multiple steel materials.""" | |
| """Module containing the class definition for a combined steel cross-section, enabling cross-sections composed of multiple steel components with potentially different materials and geometries.""" |
| steel_cross_section: SteelCrossSection, | ||
| kwargs: dict, | ||
| ) -> None: | ||
| """Test that the optional parameters can be set correctly.""" |
There was a problem hiding this comment.
The docstring says "Test that the optional parameters can be set correctly" but the test actually verifies that these parameters cannot be set via __init__ (they raise TypeError). The docstring should accurately describe what is being tested. Consider: "Test that the optional parameters cannot be set via initialization and raise TypeError."
| """Test that the optional parameters can be set correctly.""" | |
| """Test that the optional parameters cannot be set via initialization and raise TypeError.""" |
| Usage example: | ||
| ```python | ||
| combined_section = ( | ||
| CombinedSteelCrossSection() | ||
| .add_steel_cross_section( | ||
| steel_cross_section=main_steel_cross_section, | ||
| x_offset=0, | ||
| y_offset=0, | ||
| rotation_angle=0, | ||
| ) | ||
| .add_steel_cross_section( | ||
| steel_cross_section=stiffener, | ||
| x_offset=0, | ||
| y_offset=main_steel_cross_section.cross_section.cross_section_height / 2 + stiffener.cross_section.cross_section_height / 2, | ||
| rotation_angle=0, | ||
| ) | ||
| ) | ||
| ``` |
There was a problem hiding this comment.
[nitpick] The usage example references main_steel_cross_section and stiffener variables that are not defined in the example. For better clarity, consider either defining these variables in the example or adding a comment indicating they should be pre-defined, e.g.:
# Assuming main_steel_cross_section and stiffener are already defined
combined_section = (
CombinedSteelCrossSection()
...
)
…t-combinedsteelcrosssection-with-the-possibility-of-cross-sections-with-multiple-steel-grades
Description
This is the implementation of combined steel cross-sections allowing multiple steel grades in one section
(We must facilitate creating such cross-section as well. I will add a new parent issue for such features and will work on them after the refactor is done.)
The pattern is builder-composite.
I have made a questionable choice with adding a hidden method to SteelCrossSection only to be used in CombinedSteelCrossSection. Please give me feedback on that!
Fixes #824
Type of change
Checklist: