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

simplify function sigs in documentation #319

Draft
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

j-emberton
Copy link
Collaborator

Description

Closes #262
Fixes #262

This PR looks to simplify the expression of function and class type signatures in the documentation. At present, signatures can render in a verbose manner with excess information that makes parsing the information in the documentation difficult.

Simple nadarry type hints, are displayed correctly e.g.

class Calendar(dates: [ndarray])

However, type hints that relate to data that is calculated from a internal method can be excessively complex e.g.

class CoreConst(k_R: float = 8.3145, k_co: float = 209476.0, k_c_molmass: float = 12.0107, k_Po: float = 101325.0, k_To: float = 298.15, k_L: float = 0.0065, k_G: float = 9.80665, k_Ma: float = 0.028963, k_Mv: float = 0.01802, k_CtoK: float = 273.15, k_e: float = 0.0167, k_eps: float = 23.44, magnus_coef: ~numpy.ndarray[~typing.Any, ~numpy.dtype[~numpy.float32]] = , mwr: float = 0.622, magnus_option: str | None = None, water_density_method: str = 'fisher', fisher_dial_lambda: ~numpy.ndarray[~typing.Any, ~numpy.dtype[~numpy.float32]] = , fisher_dial_Po: ~numpy.ndarray[~typing.Any, ~numpy.dtype[~numpy.float32]] = , fisher_dial_Vinf: ~numpy.ndarray[~typing.Any, ~numpy.dtype[~numpy.float32]] = , chen_po: ~numpy.ndarray[~typing.Any, ~numpy.dtype[~numpy.float32]] = , chen_ko: ~numpy.ndarray[~typing.Any, ~numpy.dtype[~numpy.float32]] = , chen_ca: ~numpy.ndarray[~typing.Any, ~numpy.dtype[~numpy.float32]] = , chen_cb: ~numpy.ndarray[~typing.Any, ~numpy.dtype[~numpy.float32]] = , simple_viscosity: bool = False, huber_tk_ast: float = 647.096, huber_rho_ast: float = 322.0, huber_mu_ast: float = 1e-06, huber_H_i: ~numpy.ndarray[~typing.Any, ~numpy.dtype[~numpy.float32]] = , huber_H_ij: ~numpy.ndarray[~typing.Any, ~numpy.dtype[~numpy.float32]] = )

This PR aims to simplify expressions of the form:
var: ~numpy.ndarray[~typing.Any, ~numpy.dtype[~numpy.float32]] =

To:
var: ndarray[Any, float32] =

Through use of aliases.

Similarly, type hints relating to variables in dataclasses defined by a post init process, are displayed in the following format:

class DailySolarFluxes(lat: dataclasses.InitVar[numpy.ndarray[typing.Any, numpy.dtype[+_ScalarType_co]]], elv: dataclasses.InitVar[numpy.ndarray[typing.Any, numpy.dtype[+_ScalarType_co]]], dates: ~pyrealm.core.calendar.Calendar, sf: dataclasses.InitVar[numpy.ndarray[typing.Any, numpy.dtype[+_ScalarType_co]]], tc: dataclasses.InitVar[numpy.ndarray[typing.Any, numpy.dtype[+_ScalarType_co]]], core_const: ~pyrealm.constants.core_const.CoreConst = )

This PR aims to simplify the type hints associated with the arguments in the function/class signature to a simple expression e.g. arg: ndarray

Type of change

  • New feature (non-breaking change which adds functionality)
  • Optimization (back-end change that speeds up the code)
  • Bug fix (non-breaking change which fixes an issue)

Key checklist

  • [x ] Make sure you've run the pre-commit checks: $ pre-commit run -a
  • [ x] All tests pass: $ poetry run pytest

Further checks

  • Code is commented, particularly in hard-to-understand areas
  • Tests added that prove fix is effective or that feature works

@davidorme
Copy link
Collaborator

I'll add this to the list of built versions on RTD to make it easier to review.

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.24%. Comparing base (759c7dc) to head (70a392e).
Report is 227 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop     #319   +/-   ##
========================================
  Coverage    95.24%   95.24%           
========================================
  Files           28       28           
  Lines         1703     1703           
========================================
  Hits          1622     1622           
  Misses          81       81           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -13,6 +13,7 @@
from datetime import datetime

import sphinxcontrib.bibtex.plugin
import tuple
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@j-emberton This breaks the docs build - we can just the generic for typing so I think can delete this line.

Copy link
Collaborator Author

@j-emberton j-emberton Oct 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think its this line:

signature = signature.replace("~numpy.ndarray", "ndarray")

They seem to build ok locally when I comment these lines out.

If we want we can either tolerate the extranumpy., or try and find a work around to get the best presentation.

Copy link
Collaborator

@davidorme davidorme Oct 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is definitely failing it on my machine and RTD:

➜  docs git:(262-incorrect-argument-types--docs) ✗ make html 
Running Sphinx v7.4.7

Configuration error:
There is a programmable error in your configuration file:

Traceback (most recent call last):
  File "/Users/dorme/Library/Caches/pypoetry/virtualenvs/pyrealm-QywIOHcp-py3.11/lib/python3.11/site-packages/sphinx/config.py", line 529, in eval_config_file
    exec(code, namespace)  # NoQA: S102
    ^^^^^^^^^^^^^^^^^^^^^
  File "/Users/dorme/Research/REALM/pyrealm/docs/source/conf.py", line 16, in <module>
    import tuple
ModuleNotFoundError: No module named 'tuple'

make: *** [html] Error 2

If I delete that line, I get a "successful" build: no errors but 201 warnings! A lot of those are something going wrong with the simplify_type_hints function.

WARNING: error while formatting signature for pyrealm.constants: Handler <function simplify_type_hints at 0x105fbcd60> for event 'autodoc-process-signature' threw an exception (exception: simplify_type_hints() takes 2 positional arguments but 7 were given)

@j-emberton
Copy link
Collaborator Author

j-emberton commented Oct 7, 2024

In my attempt to strip initvar we have gone from this:

image

To this:
image

Any thoughts on this @davidorme?

In the course of my faffing with conf.py I might have changed how Sphinx/Napoleon is behaving which has resulted in the parameter descriptions being more verbose. I'll see if I can fix it.

But I'm not sure how we can strip the initvar and still keep a basic type hint for the class signature at the top. Maybe we don't need it if its documented below?

@davidorme
Copy link
Collaborator

Hmm. I'm used to seeing the types in the signature and I think that is a good overview way to see how the callable is used, but I think that's just inertia on my part and having the types with the descriptions of the parameters is equally good.

I do not like what is happening to the unpacking of the typing at all 😄

@j-emberton
Copy link
Collaborator Author

Hmm. I'm used to seeing the types in the signature and I think that is a good overview way to see how the callable is used, but I think that's just inertia on my part and having the types with the descriptions of the parameters is equally good.

I do not like what is happening to the unpacking of the typing at all 😄

Do you have 5 mins for a quick chat?

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

Successfully merging this pull request may close these issues.

Incorrect display of function/class argument types in documentation.
3 participants