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 variable referenced before assignment when using no_filtering_by_… #239

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

Conversation

tomhend
Copy link

@tomhend tomhend commented Jun 2, 2023

…endpoints, and unecessary if statement

When running Tracking with --no_filtering_by_endpoints the variables bundle_mask_ok, beginnings_mask_ok, endings_mask_ok are referenced before assignment in the first if statement below the ### Tracking ### comment.
Fixed by moving all the code for filtering_by_endpoints = True into the original if statement, and removing the if filter_by_endpoints in the original if statement underneath ### Tracking ###.
Tested Tracking both with --no_filtering_by_endpoints and without flag, with flag now runs, without produces similar tracts as before.

@wasserth
Copy link
Collaborator

Thanks for this contribution. The commit seems to contain more changes, than necessary to solve the "reference before assignment" error. E.g. the -seed and -cutoff parameter for tckgen is removed. Is this intended?

@tomhend
Copy link
Author

tomhend commented Jun 16, 2023

Hello, thanks for the comment. It's not the case that these are removed. The git compare looks to be misalligned because of the repeated code segments and the huge indentation I removed. You can see it matches lines from the iFOD tracking with lines from the FACT tracking now. If you look at the file you will see it all in place. However I haven't rigorously tested this code, it was more a suggestion I guess. If you want to be more sure you can just fix the reference before assignment.

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