-
Notifications
You must be signed in to change notification settings - Fork 2
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
Fixed python Materials enum invalid keys #214
Fixed python Materials enum invalid keys #214
Conversation
…n be accessed as enum values
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.
Fixes issue as now allows you to pull the material, however, it now removes those beginning with "_" when tabbing for materials list.
this is likely due to python marking any values beginning with |
…hen searching with tab
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.
See commented code for requested changes
LadybugTools_Engine/Python/src/ladybugtools_toolkit/external_comfort/material.py
Outdated
Show resolved
Hide resolved
LadybugTools_Engine/Python/src/ladybugtools_toolkit/external_comfort/material.py
Outdated
Show resolved
Hide resolved
LadybugTools_Engine/Python/src/ladybugtools_toolkit/external_comfort/material.py
Outdated
Show resolved
Hide resolved
will commit regex change locally as it should go after substitution Co-authored-by: James Ramsden <james.ramsden@burohappold.com>
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.
Changes worked. All materials which previously were hidden due to beginning with a _
are now showing in materials list beginning with "Material" in place of _
. Pulls successfully.
@BHoMBot check required |
@Tom-Kingstone to confirm, the following actions are now queued:
There are 146 requests in the queue ahead of you. |
@Tom-Kingstone to confirm, the following actions are now queued:
There are 72 requests in the queue ahead of you. |
@BHoMBot check ready-to-merge |
@Tom-Kingstone to confirm, the following actions are now queued:
There are 70 requests in the queue ahead of you. |
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.
Requested changes now implemented
@BHoMBot check ready-to-merge |
@Tom-Kingstone to confirm, the following actions are now queued:
There are 153 requests in the queue ahead of you. |
NOTE: Depends on
Issues addressed by this PR
Closes #213
When creating the enum, instead of calling
sanitise_string
, it calls the new methodget_identifier
which behaves similarly to sanitise_string, except it also excludes-
,.
and prepends_
when the identifier starts with a number.Test files
try to get a material using the enum as follows:
should return the material given properly.
Ensure that the Materials enum contains all the materials. (can tab through and compare to latest alpha/beta)
Changelog
Additional comments