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

mrtrix3.path.wait_for(): Fix variable ghosting #2968

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

Conversation

Lestropie
Copy link
Member

Thought that I had resolved this previously, probably when doing #2609, but in doing some other refactoring discovered that the problematic code was still in place. Maybe I fixed it some time but never pushed.

  • num_exit() was a bad name for the function given its operation; it checks for the number of files in a list that exist. But there was also an integer variable called num_exist.

  • num_in_use() function immediately got hidden behind integer variable num_in_use.

  • Function was immediately waiting 60s, rather than the intended operation of using a progressively increasing delay time up to a maximum of 60s.

The functionality should be reviewable from the code, it's pretty obviously wrong. Verifying within an existing script is hard because I can't replicate the circumstances in which it is required, but I've at least tested in an interactive Python terminal.

@Lestropie Lestropie requested a review from a team August 17, 2024 02:50
@Lestropie Lestropie self-assigned this Aug 17, 2024
@Lestropie Lestropie added this to the 3.0.5 updates milestone Sep 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant