Closed wingedRuslan closed 5 years ago
This is SO COOL @wingedRuslan! Way to go!
I'll add some comments now 😸
Hey @wingedRuslan! I've had a great evening playing with your tutorial!! Super fun 💖
I've had a big edit of the tutorial - see https://github.com/wingedRuslan/scona/pull/2 for the changes.
There are only small changes in the other parts of the code.
The plot_network_measures
code works (beautifully) but I find it really hard to read. Can you have a go at adding a bunch more documentation to it? I think I would create another function that builds the pandas data frames. That's the part that I'm really not following well when I read the code.
Have a think about whether it makes sense to have a different tutorial for the brain images. I think it would be a little clearer to have this one be for "reporting network properties" and another for "brain network visualisations"? What do you think?
Thank you so much for all the awesome work!
Thanks for the review @KirstieJane!
Do you mean to make docstring more detailed and well-explained or/and to make code to be read easily?
The plot_network_measures code works (beautifully) but I find it really hard to read. Can you have a go at adding a bunch more documentation to it?
Nice idea! I will create one in helpers.py
I think I would create another function that builds the pandas data frames.
Yeah, I agree about creating another tutorial for brain network visualisation and I will do that. I really wanted to make one today, but unexpectedly spent a lot of time on colormaps, sooorry :(
I’m not super sure why the function needs to be in helpers.py - I’m talking about building the specific data frame for plotting the nodal measures..... I think I’d keep it in visualisations.py because that’s the only place it will be called?? Happy to be convinced otherwise though.
On the documentation front, I mean both: more info in the doc string about what is happening with the function AND making the code more readable 😬
No problem on the timings!! You’ve done tonnes of great work here!
OH - and one last thing. Make sure to run the code through flake8 linting is you can. We have quite a lot of long lines at the moment ✏️
Yes, right! it makes sense :)
I think I’d keep it in visualisations.py because that’s the only place it will be called??
My motivation to add a newly created function ( that builds the specific data frame for plotting the global measures) to the helpers.py because it will make files, modules (and work?) more organized.
I mean that in visualisations.py
we have only functions to make plots and nothing else, in helpers.py
- any other functions needed for plotting ("helpers" functions that help to make plots and not only plots). But you are right, this function that builds a specific dataframe for plotting global measures will be used only once, so including in the same file as the plotting function might be better.
Speaking about more readable code, what makes my code bad in terms of readability? Do I use any bad practices? (I will ping @Islast as well) Appreciate your feedback, as it is really important and will allow me to improve the quality of the code.
I don’t think you use any bad practices at all!! I think a lot of pandas code is really hard to read! There are so many things that pandas can do that it ends up being hard to see what’s going on.
That’s why some documentation that says: here’s the information we have, in this shape, and here’s why we need it to be in a different shape (so seaborne interprets it properly) is probably the solution.
Have a google next week for pandas style guides....I haven’t seen one, and they might not exist, but that’s the only part that’s hard for me to wrap my head around. And it isn’t that your code is bad at all!
Hey @wingedRuslan! This all looks great to me! If you can merge https://github.com/wingedRuslan/scona/pull/6 then I'm totally fine for you to merge!
@Islast - you requested changes and I think they're all addressed. Would you mind taking a look and approving when you have a chance?
@KirstieJane, Thank you a lot! I would like to merge but as @Islast requested changes, her approval is needed.
Wooooo - click the big green button @wingedRuslan!! MERGE AWAY 🚀:star2:
[x] I'm ready to merge
What's the context for this pull request? (this is a good place to reference any issues that this PR addresses)
Visualisation tools
References issues #116 #115 and #119 #118
I've implemented visualisation tools for the following purposes:
1)
plot_rich_club
- for plotting the rich club values per degree along with the random rich club values created from a random network with a preserved degree distribution.Result was before comments
Result now after comments
2)
plot_network_measures
- plotting network measures values along with the random network values values created from a random networkResult
3)
plot_degree_dist
- plotting the degree distribution along with the degree distribution of an Erdos Renyi random graphResult was before comments
Result now after comments (no gap)
Kirstie's code was really helpful but mostly I've designed these functions by myself, so there are might be some features(changes) you would like to have and on the contrary not to have.
Basically, I tried to follow the Kirstie's code (like to have the same parameters to the functions), but the main implementation was either hard to reproduce or had errors while running. That's why I relied on my intuition and created functions from my perspective.
Feel free to point out me any wrong decisions I took while implementing visualization tools.
I have also tried to have the same outputs illustrated in issue #1.
The way how to use created functionalities I showed in the visualisations_tutorial.
The design of the functions and the produced figures
Documentation is updated to include a new module - visualisations.py