RCT/JX/Rule5-39

[Jiarongx-Xie]

This ruleset has not been tested.

[weilixu]

Good work and you are getting close to the finish line.
Some comments need to address.

[Jiarongx-Xie]

Good work and you are getting close to the finish line. Some comments need to address.

Thanks Weili for the comments and clarification! I thought the required_fields required_fields={ "$": ["weather", "ruleset_model_descriptions"], "$.weather": ["ground_temperature_schedule"], } could prevent the None exception. If it does not, do we still keep this field? Can you confirm this? Thank you! @weilixu

[weilixu]

Good work and you are getting close to the finish line. Some comments need to address.

Thanks Weili for the comments and clarification! I thought the required_fields required_fields={ "$": ["weather", "ruleset_model_descriptions"], "$.weather": ["ground_temperature_schedule"], } could prevent the None exception. If it does not, do we still keep this field? Can you confirm this? Thank you! @weilixu

You are correct.
If that is the case, you can directly do
ground_temperature_schedule = rpd["weather"]["ground_temperature_schedule"] Because the required_fields guaranteed the existence of these data. Using .get is confusing because .get return type is None | any
Similiarly, you should be safe to get the rmds by rmds = rpd["ruleset_model_instances"]

[Jiarongx-Xie]

Great! Thank you for the clarification!

[weilixu]

@Jiarongx-Xie when updating this rule, please change the rule id to 5-38, and rebase the pull request to RT/JG/section_5_numbering_update

[weilixu]

@Jiarongx-Xie There are some stylist comments but one of them needs to be addressed.

[weilixu]

I think you can do more explicit logic here:

any([get_opaque_surface_type(surface_b) == OST.BELOW_GRADE_WALL for surface_b in find_all(..., rpd)])

[Jiarongx-Xie]

Good catch!

[weilixu]

I think here you miss the get_manual_check_required_msg function to determine the message according to the RDS.
if ground_temperature_schedule exist, then return It cannot be determined if the ground temperature schedule for the project is representative of the project climate.
else:
A ground temperature schedule was not found for the project.

[Jiarongx-Xie]

Addressed!

[Jiarongx-Xie]

@Jiarongx-Xie There are some stylist comments but one of them needs to be addressed

Thank you Weili for the review. I have addressed these comments. @weilixu

[weilixu]

@Jiarongx-Xie There are some stylist comments but one of them needs to be addressed

Thank you Weili for the review. I have addressed these comments. @weilixu

I did not see updates, have you pushed your commits?