Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Policy on documenting "public" datasette.utils functions #1176

Closed
simonw opened this issue Jan 5, 2021 · 13 comments
Closed

Policy on documenting "public" datasette.utils functions #1176

simonw opened this issue Jan 5, 2021 · 13 comments

Comments

@simonw
Copy link
Owner

simonw commented Jan 5, 2021

https://github.com/simonw/datasette-css-properties starts like this:

from datasette import hookimpl
from datasette.utils.asgi import Response
from datasette.utils import escape_css_string, to_css_class

escape_css_string and to_css_class are not documented, which means relying on them is risky since there's no promise that they won't change.

Would be good to figure out a policy on this, and maybe promote some of them to "documented" status.

@simonw
Copy link
Owner Author

simonw commented Jan 5, 2021

Idea: introduce a @documented decorator which marks specific functions as part of the public, documented API. The unit tests can then confirm that anything with that decorator is both documented and tested.

@documented
def escape_css_string(s):
    return _css_re.sub(
        lambda m: "\\" + (f"{ord(m.group()):X}".zfill(6)),
        s.replace("\r\n", "\n"),
    )

Or maybe @public?

@simonw
Copy link
Owner Author

simonw commented Jan 5, 2021

It might be neater to move all of the non-public functions into a separate module - datasette.utils.internal perhaps.

@simonw
Copy link
Owner Author

simonw commented Jan 5, 2021

Known public APIs might be worth adding type annotations to as well.

@simonw simonw added the refactor label Jan 5, 2021
@simonw simonw changed the title Policy on documenting datasette.utils functions Policy on documenting "public" datasette.utils functions Jan 5, 2021
@simonw simonw added this to the Datasette 1.0 milestone Jan 5, 2021
@simonw
Copy link
Owner Author

simonw commented Jan 5, 2021

This needs to be done for Datasette 1.0. At the very least I need to ensure it's clear that datasette.utils is not part of the public API unless explicitly marked as such.

@simonw
Copy link
Owner Author

simonw commented Jan 6, 2021

I used from datasette.utils import path_with_format in https://github.com/simonw/datasette-export-notebook/blob/0.1/datasette_export_notebook/__init__.py just now.

@simonw
Copy link
Owner Author

simonw commented Nov 16, 2021

I'm inclined to create a Sphinx reference documentation page for this, as I did for sqlite-utils here: https://sqlite-utils.datasette.io/en/stable/reference.html

@simonw
Copy link
Owner Author

simonw commented Feb 6, 2022

Might do this using Sphinx auto-generated function and class documentation hooks, as seen here in sqlite-utils: https://sqlite-utils.datasette.io/en/stable/python-api.html#spatialite-helpers

This would encourage me to add really good docstrings.

.. _python_api_gis_find_spatialite:

Finding SpatiaLite
------------------

.. autofunction:: sqlite_utils.utils.find_spatialite

@simonw
Copy link
Owner Author

simonw commented Feb 7, 2022

I'm going with @documented as the decorator for functions that should be documented.

simonw added a commit that referenced this issue Feb 7, 2022
New mechanism for marking datasette.utils functions that should be covered by the
documentation, then testing that they have indeed been documented.

Also enabled sphinx.ext.autodoc which can now be used to embed the documented
versions of those functions.

Refs #1176
@simonw
Copy link
Owner Author

simonw commented Feb 7, 2022

New section is here: https://docs.datasette.io/en/latest/internals.html#the-datasette-utils-module

But it's not correctly displaying the new autodoc stuff:

image

@simonw
Copy link
Owner Author

simonw commented Feb 7, 2022

https://github.com/simonw/sqlite-utils/blob/main/.readthedocs.yaml looks like this (it works correctly):

version: 2

sphinx:
  configuration: docs/conf.py

python:
  version: "3.8"
  install:
  - method: pip
    path: .
    extra_requirements:
    - docs

Compare to the current Datasette one here:

version: 2
build:
os: ubuntu-20.04
tools:
python: "3.9"
sphinx:
configuration: docs/conf.py
python:
install:
- requirements: docs/readthedocs-requirements.txt

Looks like I need this bit:

python:
  version: "3.8"
  install:
  - method: pip
    path: .
    extra_requirements:
    - docs

@simonw
Copy link
Owner Author

simonw commented Feb 7, 2022

Read The Docs error:

Problem in your project's configuration. Invalid "python.version": .readthedocs.yaml: Invalid configuration option: python.version. Make sure the key name is correct.

simonw added a commit that referenced this issue Feb 7, 2022
@simonw
Copy link
Owner Author

simonw commented Feb 7, 2022

@simonw simonw closed this as completed Feb 7, 2022
@simonw
Copy link
Owner Author

simonw commented Feb 7, 2022

Here's the new test:

@pytest.fixture(scope="session")
def documented_fns():
internals_rst = (docs_path / "internals.rst").read_text()
# Any line that starts .. _internals_utils_X
lines = internals_rst.split("\n")
prefix = ".. _internals_utils_"
return {
line.split(prefix)[1].split(":")[0] for line in lines if line.startswith(prefix)
}
@pytest.mark.parametrize("fn", utils.functions_marked_as_documented)
def test_functions_marked_with_documented_are_documented(documented_fns, fn):
assert fn.__name__ in documented_fns

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant