8000 fix wrongly reporting multiple obsolete vars when only one is present in input file by clmould · Pull Request #3024 · ukaea/PROCESS · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix wrongly reporting multiple obsolete vars when only one is present in input file #3024

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Feb 8, 2024
Merged

fix wrongly reporting multiple obsolete vars when only one is present in input file #3024

merged 2 commits into from
Feb 8, 2024

Conversation

clmould
Copy link
Collaborator
@clmould clmould commented Jan 18, 2024 •
edited
Loading

Description

Closes #3020

  • added missing obsolete variables to obsolete_vars.py
  • added ValueError to report obsolete variables to replace STOP 1

Checklist

I confirm that I have completed the following checks:

  • I have justified any large differences in the regression tests caused by this pull request in the comments.
  • I have added new tests where appropriate for the changes I have made.
  • If I have had to change any existing unit or integration tests, I have justified this change in the pull request comments.
  • If I have made documentation changes, I have checked they render correctly.
  • I have added documentation for my change, if appropriate.

@clmould
fix wrongly reporting multiple obsolete vars when only one is present… 
… in input file

    - add missing obsolete variables to obsolete_vars.py
    - add ValueError to report obsolete variables to replace STOP 1
@clmould clmould linked an issue Jan 18, 2024 that may be closed by this pull request
[BUG] Multiple obsolete variables reported when only one is present in input file #3020
Closed
@clmould clmould requested a review from timothy-nunn January 18, 2024 10:39
@clmould clmould marked this pull request as ready for review January 18, 2024 10:39
timothy-nunn
timothy-nunn requested changes Jan 24, 2024
View reviewed changes
Copy link
Contributor
@timothy-nunn timothy-nunn left a comment

Choose a reason for hiding this comment

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

I like the idea of consolidating the help message and hints into obsolete_vars.py however I think the logic that generates the error messages could be improved to provide more flexibility when defining a obsolete variable that has no replacement but has a help message.

In fact, I'm not so sure stellarator_message would be printed out if you added one of the obsolete variables (from the Kallenbach model) to it.

@clmould clmould requested a review from timothy-nunn January 31, 2024 14:37
@timothy-nunn timothy-nunn merged commit a291f81 into main Feb 8, 2024
@timothy-nunn timothy-nunn deleted the 3020-bug-multiple-obsolete-variables-reported-when-only-one-is-present-in-input-file branch February 8, 2024 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timothy-nunn timothy-nunn timothy-nunn approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG] Multiple obsolete variables reported when only one is present in input file
2 participants
@clmould @timothy-nunn
0