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

Q2rCalculation: Add the output_parameters output to the spec #974

Merged
merged 1 commit into from
Oct 21, 2023

Conversation

sphuber
Copy link
Contributor

@sphuber sphuber commented Oct 19, 2023

Fixes #971

The Q2rCalculation did not declare the output_parameters output even though the Q2rParser attaches it always. This would lead to the calculation excepting because an unknown output would be attached. This was uncaught by the test because the Parser.parse_from_node method does not automatically validate the outputs. This is because the output spec that is checked is that of the calcfunction (which accepts anything) and not that of the Q2rCalculation.

The `Q2rCalculation` did not declare the `output_parameters` output even
though the `Q2rParser` attaches it always. This would lead to the
calculation excepting because an unknown output would be attached. This
was uncaught by the test because the `Parser.parse_from_node` method
does not automatically validate the outputs. This is because the output
spec that is checked is that of the `calcfunction` (which accepts
anything) and not that of the `Q2rCalculation`.
@sphuber sphuber force-pushed the fix/971/q2r-output-parameters branch from a0e273c to 04f3fba Compare October 19, 2023 08:29
@sphuber sphuber requested a review from mbercx October 19, 2023 08:36
Copy link
Member

@mbercx mbercx left a comment

Choose a reason for hiding this comment

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

Thanks @sphuber. I find it somewhat strange I hadn't run into this before, but I probably didn't run any q2r.x calculations anymore after 1c223c7 was merged.

@sphuber sphuber merged commit 7a303f9 into main Oct 21, 2023
12 checks passed
@sphuber sphuber deleted the fix/971/q2r-output-parameters branch October 21, 2023 11:39
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.

q2r "output_parameters"
2 participants