bctpy
bctpy copied to clipboard
Pythonic style
Questions regarding possible contributions and how well they would be received.
-
It seems the visualization module of BCT has not been ported to bctpy. Is this intentional? Is this planned for a future version?
-
Some names are not standardized across modules. For example,
clustering.py
usesclustering_coef_wd
, wherewd
stands for weighted directed, whereascore.py
usesassortativity_wei
for weighted networks. I know this comes from the original BCT, but I think there's a point in making this package as good-quality as possible (and that includes consistent naming conventions). -
Related to previous one. Is there interest in trying to make the package more pythonic, instead of a direct translation of BCT? For example,
bctpy [master]$ flake8 . | wc -l 546
There are 546 lines which deviate from flake8
styling guidelines. Is there interest in making bctpy adhere more to python standards?
If so, I would like to discuss the best way of setting up a PR.
-
The visualization module has been ported to bctpy. Check under utils. Certain functions were changed since the matlab plot3D had no straightforward equivalent so I implemented it with mayavi.
-
I have no strong opinion about the naming convention. Conservatively I copied the names in BCT. I would be open to changing them. For backwards compatiblity, it might be good to set up aliases for the old function names, after the new naming convention is established.
-
Adhering to various different style guidelines is not a high priority for me. I am open to PRs addressing style issues. I will happily receive any style changes that don't negatively affect the readability of the code at a glance.
-
Noted.
-
There are five functions that compute the clustering coefficient:
clustering_coef_bd(A) clustering_coef_bu(G) clustering_coef_wd(W) clustering_coef_wu(W) clustering_coef_wu_sign(W, coef_type='default')
I propose defining just one dispatcher function, which can easily handle the first four cases.
clustering_coef(A, weighted=False, directed=False)
This function would just switch over its arguments and call the correct version of
clustering_coef
.In this way, we don't touch the current functions so we have full backwards compatibility, but we can expose and encourage the use of the new dispatcher in the future.
The fifth case,
clustering_coef_wu_sign(W, coef_type='default')
can be supported by the dispatcher with one more optional parameter,clustering_coef(A, weighted=False, directed=False, coef_type='default')
which will only be used in case
weighted
anddirected
areTrue
. A little less clear than I would have hoped, but I think it still works.I propose this scheme for all other measures too. There are also four versions for
transitivity
, two foragreement
, etc. -
I'm sure by adhering to PEP8 we won't incur in any readability blunders, all the contrary. I'll prepare an example PR soon.