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

Support/respect autodoc's autodoc_preserve_defaults and autodoc_typehints_format settings #95

Open
Yoshanuikabundi opened this issue Jan 31, 2022 · 5 comments
Assignees
Labels
enhancement New feature or request

Comments

@Yoshanuikabundi
Copy link

In the last few releases, autodoc has added two settings to conf.py that are very useful:

  • autodoc_preserve_defaults: Preserves default argument values as they are in code, rather than evaluating them
  • autodoc_typehints_format: Suppresses the module names of type hints

By turning each of these on, a field signature could be transformed from something like this:

target_templates: List[Union[openff.bespokefit.schema.targets.TorsionProfileTargetSchema, openff.bespokefit.schema.targets.AbInitioTargetSchema, openff.bespokefit.schema.targets.VibrationTargetSchema, openff.bespokefit.schema.targets.OptGeoTargetSchema]] = [TorsionProfileTargetSchema(weight=1.0, reference_data=None, extras={}, type='TorsionProfile', attenuate_weights=True, energy_denominator=1.0, energy_cutoff=10.0)]

to

target_templates: List[Union[TorsionProfileTargetSchema, AbInitioTargetSchema, VibrationTargetSchema, OptGeoTargetSchema]] = [TorsionProfileTargetSchema()]

Which is much less intimidating and easier to read.

It would be amazing if autodoc_pydantic could support these options in pydantic models etc.

@mansenfranzen
Copy link
Owner

@Yoshanuikabundi Thanks for opening an issue here again 😄.

I agree, this would be a nice enhancement. To be more concretely, this would apply to pydantic field signatures specifically, right?

I will take a look into it and how to support it for newer sphinx versions.

@mansenfranzen mansenfranzen self-assigned this Jan 31, 2022
@mansenfranzen mansenfranzen added the enhancement New feature or request label Jan 31, 2022
@Yoshanuikabundi
Copy link
Author

😁

This would apply to field signatures for sure, but possibly also other signatures. It already works for methods, but having it for validators and configs and whatever else in addition to fields would be maximally consistent 😃

@mansenfranzen
Copy link
Owner

@Yoshanuikabundi I finally manage to take a closer look at your proposal. While I'm still totally convinced that shortening type hints and preserving defaults do improve documentation, I have doubts where and how to implement it.

Example

More concretely, lets consider the following example:

example.py

from pydantic import BaseModel
import io


class PydanticModel(BaseModel):
    """Doc String.

    """

    field: io.StringIO = io.StringIO()
    """Summy dummy docs"""

    class Config:
        arbitrary_types_allowed = True

    def some_class_func(self, i2: io.StringIO = io.StringIO()) -> io.StringIO:
        """Some docs"""
        pass


class PlainPython:
    """Doc String.

    """

    field: io.StringIO = io.StringIO()
    """Summy dummy docs"""

    def some_class_func(self, i2: io.StringIO = io.StringIO()) -> io.StringIO:
        """Some docs"""
        pass


def dummy_func(i: io.StringIO, i2: io.StringIO = io.StringIO()) -> io.StringIO:
    """Some docs"""
    pass

conf.py

extensions = ["sphinxcontrib.autodoc_pydantic"]

autodoc_typehints_format = "short"
autodoc_preserve_defaults = True

Results

Generating docs for example.py reveals the following behaviour with sphinx 4.4.0:

  • Class attribute field is not documented correctly for neither PydanticModel or PlainPython not respecting preserved defaults and shortened type hints.
  • Class method some_class_func is documented correctly with preserved defaults and shortened type hints.
  • Plain python function dummy_func is documented correctly with preserved defaults and shortened type hints.

Conclusion

Without taking a look at the actual sphinx implementation, it is obvious that plain sphinx autodoc does actually not respect autodoc_preserve_defaults and autodoc_typehints_format for plain python class attributes (e.g. not pydantic fields). However, functions and methods work as expected for both plain python classes and pydantic models.

In general, autodoc_pydantic wants to reuse almost everything that is already provided by sphinx autodoc. Hence, if autodoc_preserve_defaults and autodoc_typehints_format would work with plain python class attributes via sphinx autodoc, it should also work for pydantic fields with autodoc_pydantic out of the box.

Moreover, I see this as an inconsistent behaviour in sphinx autodoc that autodoc_preserve_defaults and autodoc_typehints_format are respected for function signatures but not for class attributes. Therefore, it rather belongs to sphinx autodoc than to autodoc_pydantic. Or put differently, autodoc_preserve_defaults and autodoc_typehints_format have to be marked to explicitly work for functions.

Anyway, you could argue that autodoc_pydantic could support this feature anyway. However, by doing so, we would diverge with what autodoc_preserve_defaults and autodoc_typehints_format actually mean in sphinx autodoc and autodoc_pydantic. I could agree with adding somewhat modified configuration names like autodoc_pydantic_fields_preserve_defaults and autodoc_pydantic_fields_typehints_format to make this distinction very clear. But I would still prefer to see sphinx autodoc to support this out of the box with class attributes instead of putting the logic into autodoc_pydantic.

What is your take on this?

@Yoshanuikabundi
Copy link
Author

Thanks for looking into this! Sorry, I didn't even consider the possibility that this might be autodoc's inconsistency. I definitely agree that this sounds like an autodoc problem and not an autodoc_pydantic problem. I'll make sure I can replicate your findings and then either raise an issue or PR with Sphinx. I don't think there's any need for autodoc_pydantic to provide this functionality independently, especially since I think preserving defaults involves parsing the actual source files and that would be an enormous amount of work and code to duplicate here.

Thanks!

@Timost
Copy link

Timost commented Apr 11, 2022

Hi ! Maybe this comment can help ? sphinx-doc/sphinx#10290 (comment)

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

No branches or pull requests

3 participants