Skip to content

Check the user guides and docstrings against the implementation #927

Description

@laughingman7743

Use case

While filling the missing docstrings for #882 (PR #919), the docstrings around the new ones turned out to contain claims that do not match the code. They include wrong examples, attributes that do not exist, and supported types that are not supported. The same pass found one inaccuracy in docs/usage.md (the on_start_query_execution timing on a cache_size hit), which #919 fixes. The user guides under docs/ have not been checked against the implementation as a whole. Many of their examples were written together with these docstrings, so they likely have similar problems.

Readers copy these examples and rely on the described arguments and behavior, so an inaccuracy turns into a failing call or a wrong assumption.

Docstring inaccuracies found so far

These were reported during #919 and are not yet verified one by one. Line numbers are on master 775874c.

  • pyathena/model.py:37: the AthenaQueryExecution example uses cursor._last_query_execution, which does not exist.
  • pyathena/model.py:411: the AthenaCalculationExecution "See Also" links API_CalculationSummary. The class reads GetCalculationExecution fields.
  • pyathena/async_cursor.py:29-33: Attributes: describes description as a sequence and lists rowcount. On AsyncCursor, description is a method that takes a query ID, and there is no rowcount.
  • pyathena/async_cursor.py:39, :294: the examples treat the return value of execute() as a future. It is a (query_id, future) tuple.
  • pyathena/aio/cursor.py:33, :220: async with AioConnection.create(...) lacks await.
  • pyathena/formatter.py:129: the wrap_unload example output ends in uuid//. The code builds one trailing slash. The docstring also has no Raises: for the ProgrammingError on an empty query.
  • pyathena/formatter.py:383: DefaultParameterFormatter lists time as supported. _DEFAULT_FORMATTERS has no time entry, so a time parameter raises TypeError. Either the docstring or the formatter is wrong.
  • pyathena/pandas/cursor.py:109: **kwargs is described as passed to pandas.read_csv. __init__ passes it to the base cursor.
  • pyathena/pandas/result_set.py:267-268: arraysize is described as "not used for pandas processing" (it is the fetchmany() default), and retry_config as being for S3 operations (it is also used for GetQueryResults).
  • pyathena/aio/result_set.py:72, pyathena/pandas/result_set.py:282, pyathena/polars/result_set.py:233: result_set_type_hints is described as keyed by column name. The keys can also be zero-based column indexes, and names match case-insensitively.
  • pyathena/aio/result_set.py:161: fetchone returns "a tuple"; the dict result set returns a dict. :179: fetchmany falls back to arraysize for non-positive sizes too, not only None.
  • pyathena/spark/common.py:52, spark/cursor.py:36, spark/async_cursor.py:42: Attributes: lists engine_configuration, which has no public attribute or property. spark/async_cursor.py:40 lists max_workers; check whether a public attribute exists.
  • pyathena/polars/converter.py:44: says the converter handles decimal types. Check whether the conversion mapping does, or only get_dtype().
  • pyathena/filesystem/s3_object.py:47: the S3StorageClass docstring leaves out BUCKET and DIRECTORY (:80-81).
  • pyathena/sqlalchemy/compiler.py:86-87: "TEXT, NCHAR, NVARCHAR all map to STRING" holds only without a length, and "FLOAT maps to REAL in CAST expressions" applies only to the DML cast path.

Proposed change

  1. Check the user guides against the implementation. Read each page under docs/: aio.md, api.md, arrow.md, cursor.md, filesystem.md, introduction.md, null_handling.md, pandas.md, polars.md, s3fs.md, spark.md, sqlalchemy.md, testing.md and usage.md. Check examples, argument names and defaults, return types, raised exceptions, and behavior descriptions against the code. contributing.md and index.md are out of scope unless they state behavior. Add the confirmed findings to this issue with file and line, and keep unconfirmed ones separate.
  2. Verify the docstring list above in the code, and drop or correct the entries that do not hold.
  3. Fix the confirmed findings. Decide here whether the docstrings and the guides go in one PR or two, depending on the number of findings. Where the code is wrong rather than the documentation (for example, the time formatter), file a bug or feature issue instead of documenting the defect.

This builds on #919 (docstrings for every public API). The docstring fixes touch the same files, so they should start after #919 is merged.

Validation plan (if implementing)

  • just lint, just docs lint, and a Sphinx build (sphinx-build -b html docs <out>) with no new warnings against master.
  • Run the corrected examples that need no AWS access (for example, the formatter output) and confirm their output. Examples that need Athena are checked against the code, not run, unless a finding depends on service behavior.
  • Documentation-only changes need no AWS integration tests.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions