Skip to content

Conversation

bartfeenstra
Copy link
Contributor

@bartfeenstra bartfeenstra commented Mar 14, 2025

This PR:

  • Adds type hints to everything in gramps.gen.lib.primaryobj, as well as any methods overridden in subclasses
  • Uses typing.override to help mypy ensure subclasses override methods correctly, and for them to inherit the parents' docstrings (hence the large number of removals).

@Nick-Hall
Copy link
Member

This is a Python 3.12 feature so we may need to install typing_extensions for this to work in older versions.

Yes. The code should run on python 3.10 in the master branch.

@bartfeenstra
Copy link
Contributor Author

This is a Python 3.12 feature so we may need to install typing_extensions for this to work in older versions.

Yes. The code should run on python 3.10 in the master branch.

Do we want CI builds for that so that people like me don't accidentally break things?

@bartfeenstra bartfeenstra force-pushed the typed-primaryobj branch 2 times, most recently from b06d8d1 to 4117589 Compare March 14, 2025 18:25
@Nick-Hall
Copy link
Member

The CI is running 3.10 in the master branch and I run 3.13 locally.

@bartfeenstra
Copy link
Contributor Author

Oh right, I confused the linter action (which passed) with the CI one which is the one that actually runs mypy (soz!). I updated the PR to use typing_extensions so this should pass on 3.10 now.

@bartfeenstra bartfeenstra force-pushed the typed-primaryobj branch 3 times, most recently from 96c8d60 to e2464be Compare March 14, 2025 19:07
@kulath
Copy link
Member

kulath commented Aug 6, 2025

Why are the docstrings mostly removed or truncated?

Does the documentation tool use the type hints?

(Sorry if I have not understood the significance of the typing.override).

@bartfeenstra
Copy link
Contributor Author

@kulath Fair question! I erroneously assumed this was part of the typing.override specification, but in fact it's only a suggested effect for libraries. For example, Sphinx will automatically use the super method's docstring when decorating a subclass's method with typing.override. The practical benefit of this is that it saves an enormous amount of time keeping (often duplicate) docstrings in sync.

https://peps.python.org/pep-0698/#set-override-true-when-possible says:

As a concrete example, a runtime library could check __override__ in order to automatically populate the __doc__ attribute of child class methods using the parent method docstring.

@kulath
Copy link
Member

kulath commented Aug 7, 2025

@bartfeenstra Thanks for the response.

I note that the quoted comment says "could". It also talks about using a run-time library, whereas I am thinking about the generated documentation.

I am not sure whether Sphinx will actually generate the documentation correctly. I am looking at https://gramps-project.org/api_3_3_x/api.html (I know this is out of date, but I didn't take the time to find the current doc.)

I wonder whether it would be possible for you to generate the documentation from this modified code, and se whether it correctly uses the typing.override ?

Edit to add: This is the current documentation https://gramps-project.org/docs/gen/gen_lib.html

@bartfeenstra
Copy link
Contributor Author

Sphinx does this, yeah. See https://betty.readthedocs.io/stable/betty.ancestry.person/#betty.ancestry.person.Person.dump_linked_data for an example of an overriden method that inherits its parent's docstring.

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

Successfully merging this pull request may close these issues.

4 participants