-
Notifications
You must be signed in to change notification settings - Fork 41
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
changing name of segmentation map flux keyword #717
base: master
Are you sure you want to change the base?
Conversation
I think the minimum_signal and minimum_signal_units are correct in this case. You can see in the call to yaml_generator, that it contains: I see that there is a typo in that minimum_signal is misspelled, but I think that's the only change that's necessary. |
Hmm, maybe I'm looking at the wrong branch or misinterpreted code but when I go here: line 136 says: And line 326 says: |
Ah got you! I didn't read all the way down at the bottom of the page where you call |
Right. minimum_signal and minimum_signal_units are just random variable names that are used as inputs. Maybe I should have put a call to yaml_generator in each section of that page, to cut down on confustion. |
Ok, I updated this pull request to use the same segmap_flux_limit in both the description and the call to yaml_generator.SimInput for lazy people like me who can't read the whole page for find the correct docstring for the function. Up to you how to you see fit to proceed |
Actually, I think the only change needed is: |
Or yeah, you could change all instances of minimum_signal to segmap_flux_limit, and minimum_signal_units to segmap_flux_limit_units. |
Maybe for lazy people who don't read the whole thing like me, but you are correct and fair with segmap_flux_limit=minmum_signal -> segmap_flux_limit=minimum_signal. |
Updating the docs to the right keyword to use in the yaml generator for setting a flux limit for the segmentation map threshold.