Skip to content
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

Fix issue #130 #131

Merged
merged 2 commits into from
Nov 29, 2023
Merged

Fix issue #130 #131

merged 2 commits into from
Nov 29, 2023

Conversation

lohedges
Copy link
Contributor

This PR closes #130 by allowing the perturbable_constraint dynamics option to contain hyphens.

  • I confirm that I have merged the latest version of devel into this branch before issuing this pull request (e.g. by running git pull origin devel): [y]
  • I confirm that I have added a changelog entry to the changelog (we will add a link to this PR as part of the review): [y]
  • I confirm that I have permission to release this code under the GPL3 license: [y]

Suggested reviewers:

@chryswoods

chryswoods
chryswoods previously approved these changes Nov 27, 2023
Copy link
Contributor

@chryswoods chryswoods left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I think this highlights that the way I did this for constraint was error prone. I think it should be that there is a replace("-", "_") after the simplify, so that we can match any mix of hyphens or underscores.

@lohedges
Copy link
Contributor Author

You do sanitize in the Python layer, i.e. everything gets converted to underscores. Are there situations where the C++ is being called directly?

@chryswoods
Copy link
Contributor

Not currently, but it is better to be safe than sorry. There would be an edge case where someone sets this as a map parameter that bypasses the sanitation.

Signed-off-by: Christopher Woods <[email protected]>
Copy link
Contributor

@chryswoods chryswoods left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good

@chryswoods chryswoods merged commit f00b9b3 into devel Nov 29, 2023
0 of 5 checks passed
@chryswoods chryswoods deleted the fix_130 branch November 29, 2023 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] perturbable_constraint validation doesn't check for underscores
2 participants