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 non-returned value #12

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix non-returned value #12

wants to merge 1 commit into from

Conversation

pbitzer
Copy link
Contributor

@pbitzer pbitzer commented Oct 21, 2021

If a state variable was not present, or had a bad/NaN value, an exception would be raised. In this case, the _previous_data attribute would not be updated, and the error would persist at future steps. This makes sure previous data is update AND make sure the input data is returned for other steps in the brokkr chain.

If a state variable was not present, or had a bad/NaN value, an exception would be raised. In this case, the `_previous_data` attribute would not be updated, and the error would persist at future steps. This makes sure previous data is update AND make sure the input data is returned for other steps in the brokkr chain.
@CAM-Gerlach CAM-Gerlach added bug Something isn't working and removed bug Something isn't working labels Oct 21, 2021
Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

Choose a reason for hiding this comment

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

Actually, since the try block this finally block attaches to already has an except Exception that doesn't terminate execution (and no else block), in case of an error caught by it, execution just continues to these two lines anyway if any error occurs without this change. The only scenario this makes any difference in is if a KeyboardInterrupt or similar BaseException occurs during execution, in which case you want to terminate immediately and which is already caught and handled anyway by the custom signal/interrupt handler. So I'm not sure this PR really makes any difference?

@CAM-Gerlach
Copy link
Member

@pbitzer should we close this?

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