Closed ClaudMor closed 2 weeks ago
Thank you for catching these! I'll make a pass over all tutorials and ensure they are completely ported to python3. While I'm at it, I'll also translate the tutorials into Jupyter Notebooks (this was on my TODO list anyway). This should also take care of some of the other issues you identified.
You might try adding a Github action to test the notebooks (For example, see our Github action in XGI)
RST Tutorials reviewed for python2 remnants + other update needs:
Thanks @dataspider for translating this tutorial to python 3.
Anyway, I suggest changing the following line:
print(map(nx.number_connected_components,models.er(1000,3*[0.005]).A.values()))
to
print(map(nx.number_connected_components,pymnet.models.er(1000,3*[0.005]).A.values()))
to avoid an Exception being thrown. Moreover, its output is still:
<map object at 0x0000013B1166CEB0>
instead of the documented
[10, 9, 5]
@ClaudMor Thanks, I manually fixed this tutorial for now, but tutorials will get a notebook revamp at some point soon.
My bad (some of my local changes got lost in translation) – thanks @arashbm for fixing this (and yes, jupyter notebook versions are on the way)!
Tutorials are now migrated to Python 3 and translated (where applicable) into Jupyter Notebooks, which are included in the docs via MyST_NB; see, e.g., here.
@nwlandry Thank you for the pointer to your GitHub action for testing notebooks – I am hoping to do cell-by-cell output comparisons and integrate these into our suite of unit tests (stub here) but have yet to set make this work in our GitHub-action test environment. But since the docs now show the results of true notebook executions, I'll treat this as a separate issue and close this one.
@dataspider That sounds like a plan and certainly not essential for the review. Just a suggestion for future development.
Dear authors,
I noticed that in the tutorial on Using NetworkX functions, some residuals from Python 2 are present:
Moreover the last line, when ported to python 3, does not return
[9, 11, 8]
as reported: