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.

Sign up for GitHub

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

Update view_corr_mat in visualisation commands and Update the introduction page in docs #108

Merged
merged 8 commits into from
Apr 18, 2019
Merged

Update view_corr_mat in visualisation commands and Update the introduction page in docs #108

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
0cb64ab
Try to link to autodoc content in the introduction.rst
wingedRuslan Apr 2, 2019
86ff8d6
Add documentation to the introduction page
wingedRuslan Apr 7, 2019
1500c32
Add documentation to the introduction page, ready for PR
wingedRuslan Apr 2, 2019
5412842
Merge branch 'master' of https://github.com/wingedRuslan/scona
wingedRuslan Apr 7, 2019
9d083dc
Update view_corr_mat in visualisation commands to accept corr_mat as …
wingedRuslan Apr 9, 2019
d93b9fe
Add support of numpy.array as an input for view_corr_mat and add rais…
wingedRuslan Apr 16, 2019
d9ed917
Check that corr_mat is n x n matrix, otherwise raise Error
wingedRuslan Apr 16, 2019
f9fe78c
Explain corr_mat argument to view_corr_mat() more precisely
wingedRuslan Apr 18, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 18 additions & 3 deletions docs/source/introduction.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,25 @@ You can install scona directly from the GitHub repository::

If you want to edit scona it's recommended that you pass the ``-e`` flag to ``pip`` to install the package editably.

Getting Started
KirstieJane marked this conversation as resolved.
Show resolved Hide resolved
---------------

We have automatically generated `docstring <https://github.com/WhitakerLab/scona/blob/master/DEVELOPMENT_GUIDE.md#writing-docstrings>`_ documentation and here's how to navigate to it.

See all docs organized in the alphabetical order:
* :ref:`genindex`

See the structure of the package:
* :ref:`modindex`

See the submodules page:
* :ref:`ref-subpackages-label`

| Besides, you can type any function into the **search bar** and come up with some results.
| Alongside this documentation scona has some jupyter notebook `tutorials <https://github.com/WhitakerLab/scona/tree/master/tutorials>`_.

Finding Help
------------
If you have questions or want to get in touch, you can join our `gitter lobby <https://gitter.im/WhitakerLab/BrainNetworksInPython>`_, tweet `@Whitaker_Lab <https://twitter.com/Whitaker_Lab>`_ or email Isla at [email protected].

Getting Started
---------------
Alongside this documentation scona has some jupyter notebook `tutorials <https://github.com/WhitakerLab/scona/tree/master/tutorials>`_.

2 changes: 2 additions & 0 deletions docs/source/scona.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
scona package
=============================

.. _ref-subpackages-label:

Subpackages
-----------

Expand Down
33 changes: 29 additions & 4 deletions scona/scripts/visualisation_commands.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,22 +54,47 @@ def rescale(fname, suff='png'):
# And you're done!


def view_corr_mat(corr_mat_file,
def view_corr_mat(corr_mat,
output_name,
cmap_name='RdBu_r',
cost=None,
bin=False):
''' This is a visualisation tool for correlation matrices'''
'''
This is a visualisation tool for correlation matrices

Parameters
----------
corr_mat : :class:`pandas.DataFrame` or :class:`str`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic makes sense to me! Thanks for clearly explaining your thought process in the PR 😸

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also agree with your reasoning here, fewer arguments is great, and your changes to the docstrings are very clear.

corr_mat could be a DataFrame object to represent a correlation matrix
or a string object - Path to the File, containing the matrix.
output_name : :class:`str`
the name of the file you want to save your visualization
of correlation matrix to in.
cmap_name : string or Colormap, optional
A Colormap instance or registered colormap name.
The colormap maps scalar data to colors. It is ignored for RGB(A) data.
Defaults to 'RdBu_r'.

Returns
-------
The visualization of the correlation matrix is saved in the file.

# Read in the data
M = np.loadtxt(corr_mat_file)
'''

# If cost is given then roughly threshold at that cost.
# NOTE - this is not actually the EXACT network that you're analysing
# because it doesn't include the minimum spanning tree. But it will give
# you a good sense of the network structure.
# #GoodEnough ;)

if isinstance(corr_mat, str):
M = np.loadtxt(corr_mat) # Read in the data
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feels like we should add a test for this, what do you think @wingedRuslan?

If that feels a bit too much then we can just open an issue noting that we should check the dimensions of the file when we load it in. Up to you and @Islast ✨

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can leave np.loadtxt to do the heavy lifting on properly importing data. Or did you mean it's worth checking the file isn't too large before trying to import it? That's something I've never really considered

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry folks! Massively unclear on my part.

The test that @wingedRuslan has added further down to check that the data is square is what I was thinking of, not anything about reading in the data properly 😬

elif isinstance(corr_mat, pd.DataFrame):
KirstieJane marked this conversation as resolved.
Show resolved Hide resolved
M = corr_mat.to_numpy() # Convert the DataFrame to a NumPy array
else:
print("Please provide correlation matrix as pandas.DataFrame object or as a path to the file containing the matrix")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like type checking a whole lot. My go to for handling these cases is raising a type error. This is nice because printed output can often get lost. In this case raising an exception would look like:

else:
    raise TypeError("corr_mat argument must be a pandas.DataFrame object or as a path to the file containing the matrix")

My reasoning for doing things this way is that if this command is run with a whole load of other code, it's easy for a printed message to get lost. Raising an error message aborts the execution of code (unless there is an exception for it specified) and the printed output will end with

File: "somefile", line somenumber, 
TypeError  corr_mat argument must be a  pandas.DataFrame object or as a path to the file containing the matrix

which gives the user a lot of useful information to help debug

If you come across a case when you want to convey some information to the user about how the code is being executed, but you don't want to abort the code, raising a warning is another good trick.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You've got a point! 👍
I will adjust the type checking by raising a type error.

return

if cost:
thr = np.percentile(M.reshape(-1), 100-cost)
M[M<thr] = 0
Expand Down