-
Notifications
You must be signed in to change notification settings - Fork 3
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
Refactor functioning.jl
.
#79
Comments
Yupe. What is the purpose/scope of this file btw? Is it clearly distinct from the measures eventually implemented in the other measures package? |
Yes the metrics written in this file are different from the one implemented in the package of stability metrics (if it is the one you talk about). Actually, the metrics written in this file are more related to the network structure than to its stability. |
I see, well, better renaming the file to something containing at least the keyword |
Something like |
He-hee, blurry indeed :) Well, I'd advise to base your choice on this very kind semantic considerations (before vs. after simulation, user vs. dev utils, ..), but also acknowledge that this is unstable yet and that the files may be renamed/merged/split in the future : so don't overthink this and keep it simple and modular so as to ease future changes 0:) |
(wops, sorry I'm just realizing that |
I was also thinking about returning species biomasses in the What do you think? |
If it's ok for you maybe it's best that you do the first refactoring pass, and then, Iago and me can do a second pass by reviewing your new code. Does it seem good to you? For the naming, do you mean that you want that all analysis functions (working on the output of |
I agree with you, may be there is no need for such prefix! |
I don't have a particular opinion regarding naming functions in this file because I'm not exactly confident what they mean, what the user shall invoke them for, or what semantic unity makes them all live within the same file. I'll trust you on this one guys :) What I can approve, however, is that the temptation to "prefix all functions with # Instead of:
general_A() = ...
general_B() = ...
species_B() = ...
species_C() = ...
# Prefer
A() = ...
B() = ...
module Species
B() = ..
C() = ..
end
# That you use with:
A()
B()
Species.B()
Species.C() .. or something. This is very in accordance with the last item on the Zen of Python :P However, keep in mind the 5th item (higher-rank, higher priority): |
I refactored all the metrics (function naming, arguments, doc), everything should be cleaner now. I might ask what should be on the Christmas list (a bit late) now for us as output measures! |
Great! I'm sorry I'm a bit lost, on which branch are your changes (so I can have a quick look 😇)? And is the branch pushed? |
The branch is pushed and is this one: https://github.com/BecksLab/BEFWM2/tree/simulation_output_utils |
Now in #87 🎆 |
Closed by #87. |
Refactoring the
functioning.jl
IMO some part of this script can be re-written to make it more clear/short and also some part are not consistent with the rest of the package (use ofreturn
, docstring not formatted exactly as advise by the Julia official documentation, etc.)The text was updated successfully, but these errors were encountered: