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

[GeoMechanicsApplication] GetVariableBasedOnString is now emitting a warning rather than an error in set_parameter_field_process #13011

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

aronnoordam
Copy link
Member

GetVariableBasedOnString is now emiting a warning rather than an error in set_parameter_field_process

Before, an error was emited when a variable like DENSITY_SOLID, which is not exported tot the python api, is requested

this error is only required when wanting to generate a parameter field with a python function

But the function which raises the error is still required in the init

@aronnoordam aronnoordam changed the title [GeoMechanicsApplication] GetVariableBasedOnString is now emiting a warning rather than an error in set_parameter_field_process [GeoMechanicsApplication] GetVariableBasedOnString is now emitting a warning rather than an error in set_parameter_field_process Jan 15, 2025
@mcgicjn2 mcgicjn2 requested review from rfaasse and avdg81 January 16, 2025 14:28
Copy link
Contributor

@rfaasse rfaasse left a comment

Choose a reason for hiding this comment

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

Thank you for addressing this issue and adding clear tests! I have an idea for a different solution which would require less code changes if I understand the issue correctly. Let me know what you think!
I think it is a good idea anyway though to rename the branch to geo/ such that the TC and sonarcloud pipelines also run.


# Check that a warning was raised
assert len(w) == 1
assert issubclass(w[-1].category, UserWarning)
Copy link
Contributor

Choose a reason for hiding this comment

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

We've had some issues in the past that sonarcloud doesn't recognize the assert and therefore could flag the indexging w[-1] as a 'reliability' issue. Would it be possible to rename this branch to geo/ such that the sonarcloud analysis as well as other pipelines will be run? E.g. github doesn't run the validation tests (only small/nightly)

@aronnoordam
Copy link
Member Author

#13011

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants