Issue #60 - Ensure docstrings adhere to PEP-8/Google Python Style Guide - #70
Merged
Conversation
…er adhere to PEP-8 standards, as well as to conform to the Google Python Style Guide (in particular the formatting for docstrings). Additionally, used `cython-lint` tool on `graph.pyx` to help find these warts: ``` cython-lint --max-line-length 80 planarity/full/graph.pyx ``` Note: I left the max line length as 80 rather than 79 because there's too many instances where it would just be too fussy to line-break and it's not like the single column matters that much. Also recompiled extensions to produce .c files using Cython 3.2.9
…recompiled cythonized extension `.c` files with Cython 3.2.9. Added codespell to `requirements.txt` because I ran it locally to ensure no typos made. Used `cython-lint --max-line-length 80 <filename>` and went over each file to remediate as many long lines and trailing whitespace issues as was sensible. After running `pip install .` in a Python 3.14.6 virtual environment, ran `pytest` and all tests passed (no changes to `classic` package so this makes sense, but still shows the whole package is installed and runs). Ran through `examples/full/` test scripts. Ran EAPS-T `test_all_graphs.py` and `test_table_generation_with_numInvalidOKs.py` to regenerate test tables for N=8` and compared to existing tables on EAPS (which uses old subprocess-based approach), e.g.: ``` icdiff -W -s ../output/tables/8/n8.mALL.4.out.txt ~/git/edge-addition-planarity-suite-fork/TestSupport/tables/8/n8.mALL.4.out.txt ```
…on Style Guide docstring recommendations)
To generate these docs, when at the root of the planarity project, run:
sphinx-apidoc -o doc/source planarity
Then:
sphinx-build -M html doc/source doc/build
OR from the doc directory, run
make html
If you wish to generate a .pdf, replace "html" with "latexpdf".
Need to figure out how to publish these docs to GitHub Pages. Note that I opted to not include the compiled files, since these should be automatically regenerated from the actual docstrings.
…todoc defaults are not behaving as expected. However, we now have a very simple way to build the docs without having to manually run sphinx-apidoc before sphinx-build:
To generate these docs, when at the root of the planarity project, run:
sphinx-build -M html doc/source doc/build
OR from the doc directory, run
make html
… locally by: 1. Cloning the repo 2. Setting up a fresh virtual environment by running `python3.14 -m venv .venv` and then activate it 3. Installing requirements.txt by `pip install -r requirements.txt` 4. `cd doc && make html` Also, for now, ignore the build artifacts using .gitignore.
Can't figure out how to get rid of module-level `__reduce_cython__` and `__setstate_cython__` holders... They're being skipped for the Classes (did some tests by adding debug statements to `conf.py` to try to see what was going on) The reason why __cinit__ and __dealloc__ docstrings aren't being promoted to the autogenerated docs is, according to https://cython.readthedocs.io/en/latest/src/userguide/special_methods.html#docstrings, as follows: "You can place a docstring in the source to serve as a comment, but it won’t show up in the corresponding __doc__ attribute at run time. (This seems to be is a Python limitation – there’s nowhere in the PyTypeObject data structure to put such docstrings.)"
…th notation in `gp_ContractEdge()`.
…onfiguration. I still can't exclude the module-level `__reduce_cython__()` nor `__setstate_cython__()` functions from the final generated html pages. It seems like autodoc was already correctly excluding those methods on the *classes*, but for some reason some other part of the pipeline is including these members. Increased the `toctree` `maxdepth` to 7 so that we can navigate to the class documentation from the `index.rst`.
…irtual environment site-packages rather than having to separately build_ext.
john-boyer-phd
previously approved these changes
Aug 26, 2026
john-boyer-phd
left a comment
john-boyer-phd
left a comment
Member
There was a problem hiding this comment.
This is looking great! Clearly, in hindsight, it wasn't going to work to just make a single first docstring sentence for various functions when the real goal was to end up with an API doc. Way to go!
…their docstrings do not get promoted to the final documentation tree.
john-boyer-phd
approved these changes
Aug 26, 2026
john-boyer-phd
left a comment
john-boyer-phd
left a comment
Member
There was a problem hiding this comment.
Looks great. I really like that depth 8 setting because I like the clickable table of contents up front, not just as an index.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #60
Type of change
Please check only relevant options:
inttoNoneforGraphclass methodgp_ExtendWith_DrawPlanar()Changes
Added
doc/make.bat- autogenerated bysphinx-quickstartto build docs on WindowsUpdated
requirements.txt- Addedcodespell(ran it locally to ensure no typos had been made) andcython-lint(ran it locally on all.pyxand.pxdfiles in theplanarity.fullsubpackage to remediate as many long lines and trailing whitespace issues as was sensible:cython-lint --max-line-length 80 <filename>)cython-lintis 88 as per the PyPI page..pyxand.pxdfiles to better adhere to PEP-8 standards, as well as to conform to the Google Python Style Guide (in particular the formatting for docstrings).planarity/full/graph.pyx- Changed API forGraphclass methodgp_ExtendWith_DrawPlanar()so that it no longer returnsresultso that it matches the other "extend with" methods.pip install .to refresh.cfiles using Cython 3.2.9Removed
docdirectory from previous documentation generation withsphinxcirca 2011Testing
Ran
pip install .in a Python 3.14.6 virtual environmentRan
pytestand all tests passed (no changes toclassicpackage so this makes sense, but still shows the whole package is installed and runs).Ran through
examples/full/test scripts.Ran EAPS-T
test_all_graphs.pyandtest_table_generation_with_numInvalidOKs.pyto regenerate test tables for N=8` and compared to existing tables on EAPS (which uses old subprocess-based approach), e.g.:From the
docdirectory, ranmake htmlandmake latexpdfto automatically generate the documentation from the docstrings. Using thenapoleonextension means that we should be validating the docstring formatting (although I tried to delete a period from the end of one of the components and it didn't squawk at me :( )Investigation into how to ensure docstrings adhere to Google Python style
TODO: use a Google docstring parser tool to ensure manual formatting has been done correctly. For example:
darglink(no longer maintained; perhaps usedarglink2)pydoclint- Using
edit: boooo,pydoclintconfig options to populatepydoclint_config.tomlto help run the tool locally (see 3. Specifying options in a configuration file)pydoclintdoesn't have Cython support! This will be useful for checking docstrings in EAPS-T and from theexamples/scripts, but I think we might be out of luck...