Skip to content

Documentation branch#38

Open
rpoudel1 wants to merge 8 commits into
fiuneuro:masterfrom
rpoudel1:documentation_branch
Open

Documentation branch#38
rpoudel1 wants to merge 8 commits into
fiuneuro:masterfrom
rpoudel1:documentation_branch

Conversation

@rpoudel1

Copy link
Copy Markdown
Member

Closes # .

Changes proposed in this pull request:

Comment thread brainconn/centrality/centrality.py Outdated
Cs = np.real(np.dot(vecs * vecs, np.exp(vals)))
return Cs # imaginary part from precision error

def Leverage_centrality:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You need to define functions with parentheses, even if they're empty. We also need to include the word "pass" when the function isn't running yet.

So instead of

def Leverage_centrality:
    """Documentation...
    """

we should have:

def Leverage_centrality():
    """Documentation...
    """
    pass

Comment thread brainconn/centrality/centrality.py Outdated

"""

def Delta_centrality (G):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Function names should be lower snake case, so Delta_centrality --> delta_centrality, Degree_centrality --> degree_centrality, Leverage_centrality --> leverage_centrality.

Comment thread brainconn/centrality/centrality.py Outdated
betweenness centrality participate in a large number of shortest paths.



Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We also don't want these extra spaces here.

@tsalo

tsalo commented Aug 16, 2018

Copy link
Copy Markdown
Member

These changes look good, but I'll hold off on merging the PR until we consolidate some functions and address the failing tests in other work.

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