-
Notifications
You must be signed in to change notification settings - Fork 34
Knauer US-11518: Types For Timber Design - MoistureClass and ServiceCondition #252
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
Conversation
|
This PR has Quantification details
Why proper sizing of changes matters
Optimal pull request sizes drive a better predictable PR flow as they strike a
What can I do to optimize my changes
How to interpret the change counts in git diff output
Was this comment helpful? 👍 :ok_hand: :thumbsdown: (Email) |
| member_sets (str): Assigned Member Sets | ||
| surfaces (str): Assigned Surfaces | ||
| surface_sets (str): Assigned Surface Sets | ||
| moisture_class (enum): Timber Moisture Class |
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.
Description should be Timber Moisture Class Type Enumeration
| surfaces (str): Assigned Surfaces | ||
| surface_sets (str): Assigned Surface Sets | ||
| standard (int): Code Number | ||
| moisture_service_condition (enum): Timber Service Condition |
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.
description should be Timber Moisture Service Condition Type Enumeration
| ''' | ||
| OFFSET_TYPE_XY, OFFSET_TYPE_XZ, OFFSET_TYPE_YZ = range(3) | ||
|
|
||
| class TimberMoistureClassMoistureClass(Enum): |
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.
TimberMoistureClassType is better class name
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.
I'm with Heet on this one.
| ''' | ||
| TIMBER_MOISTURE_CLASS_TYPE_1, TIMBER_MOISTURE_CLASS_TYPE_2, TIMBER_MOISTURE_CLASS_TYPE_3 = range(3) | ||
|
|
||
| class TimberServiceConditionsMoistureServiceCondition(Enum): |
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.
TimberMoistureServiceConditionType is better class name
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.
I'm with Heet on this one.
| surface_sets (str): Assigned Surface Sets | ||
| standard (int): Code Number | ||
| moisture_service_condition (enum): Timber Service Condition | ||
| temperature (enum): Timber Service Conditions Temperature |
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.
it would be good to add Enumeration at the end of description for variable type (enum)
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.
Hi,
please look at branch MichalOndrej-timberServicesAndMoisture and carefully compare the tests. There is a way to execute the tests properly even if the setters are not in the WS.
| clientObject.treatment = treatment_csa | ||
|
|
||
| # Service Condition if Standard NDS(USA) - Service Moisture Conditions, Treatment and Temperature | ||
| if standard == 6579: |
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.
'elif' instead of 'if' should be used. Same with line 91. These statements are mutually exclusive.
| clientObject.surface_sets = ConvertToDlString(surface_sets) | ||
|
|
||
| # Service Condition if Standard CSA - Moisture Service Conditions and Treatment | ||
| if standard == 6336: |
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.
Common code could be separated to enhance readability and maintainability.
parameters = {
"current_standard_for_combination_wizard": standard,
"activate_combination_wizard_and_classification": True,
"activate_combination_wizard": True,
"result_combinations_active": True,
"result_combinations_parentheses_active": False,
"result_combinations_consider_sub_results": False,
"combination_name_according_to_action_category": False}),
# Service Condition if Standard CSA - Moisture Service Conditions and Treatment
if standard == 6336:
LoadCasesAndCombinations(params = parameters, ....
| ''' | ||
| OFFSET_TYPE_XY, OFFSET_TYPE_XZ, OFFSET_TYPE_YZ = range(3) | ||
|
|
||
| class TimberMoistureClassMoistureClass(Enum): |
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.
I'm with Heet on this one.
| ''' | ||
| TIMBER_MOISTURE_CLASS_TYPE_1, TIMBER_MOISTURE_CLASS_TYPE_2, TIMBER_MOISTURE_CLASS_TYPE_3 = range(3) | ||
|
|
||
| class TimberServiceConditionsMoistureServiceCondition(Enum): |
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.
I'm with Heet on this one.
| @@ -0,0 +1,70 @@ | |||
| from RFEM.initModel import Model, clearAttributes, deleteEmptyAttributes, ConvertToDlString | |||
| from RFEM.enums import TimberMoistureClassMoistureClass | |||
| from RFEM.LoadCasesAndCombinations.loadCasesAndCombinations import LoadCasesAndCombinations | |||
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.
Unused import.
Description
The two functions set_timber_moisture_class and set_timber_service_conditions are implemented here. Since there are some issues with setting the right code, a thorough instruction can be found in every test file on how to test. Because of this reason the pytest.skipif function was added, so it doesn't interfere with existing tests.
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Test Configuration:
Checklist: