networkx / nx-guides

Examples and Jupyter Notebooks about NetworkX
https://networkx.org/nx-guides/
Creative Commons Zero v1.0 Universal
198 stars 107 forks source link

Improve consistency #98

Closed 20kavishs closed 1 year ago

20kavishs commented 1 year ago

In the NetworkX community meeting this week (on 3/21/23) I asked about high priority improvements for nx-guides.

@rossbar mentioned making the notebooks more consistent for a start

I standardized the notebooks by doing the following (this is my first contribution, so wanted to start small!)

Any other consistency fixes?

20kavishs commented 1 year ago

Also as a side note...should we rename (and change the wording in) the Euler's Algorithm notebook to be about Eulerian paths and circuits? I don't think there is an explicit "Euclid's algorithm"...it's mostly about Eulerian paths and circuits and how to find them.

Just a small semantic issue, I can take care of it quickly if it is a good idea.

Also...any insight on how to pass the build-docs checks? Looking at the failing tests, they seem to be quite odd (i.e. that 2 == 3).

navyagarwal commented 1 year ago

@20kavishs

Hey, I wanted to drop in a couple of suggestions that might help with making the guides more consistent in the long run -

  1. Having a template notebook that contains the required sections for each guide like references, a standard introduction with links to Networkx docs, imports etc.
  2. Having a guidelines document that lists the recommended practices for notebooks like adding code comments, using the Networkx vocabulary for certain keywords etc.
  3. We can link both these documents to a contributing guide
20kavishs commented 1 year ago

@navyagarwal

Good ideas, thanks for your thoughts!

I think the template notebook might be a bit much as of now...I feel that it might clutter nx-guides. I like the other two points though.

Instead of making a new guidelines document, we can just add a section to the contributor's guide already made by @dtekinoglu #81. This way potential contributors can just refer to one document.

I'm working on it right now

Edit: I put it in PR #99

@rossbar @MridulS

20kavishs commented 1 year ago

Resolved suggestions and added import packages and references section to LCA notebook, thanks for the feedback! @rossbar @MridulS

20kavishs commented 1 year ago

Addressed feedback from @dschult, thank you!

I'm struggling a bit with making the CI happy. Pre-commit tests are passing, but the nbval tests are not.

pytest --nbval-lax --durations=25 content/ is not collecting any tests for some reason, and I'm having the same issue locally.

Any thoughts on how to help the CI? @MridulS @rossbar @dschult

20kavishs commented 1 year ago

I'll aim for option 1, if the other reviewers agree?

dschult commented 1 year ago

Yes -- let's go ahead and separate these two parts of this PR.

dschult commented 1 year ago

OK.. I think this improve consistency PR is ready for review. The traversal has been separated and the notebook tests are working.