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

Add domain_types sphinx extension #110

Merged
merged 6 commits into from
Jan 25, 2024
Merged

Add domain_types sphinx extension #110

merged 6 commits into from
Jan 25, 2024

Conversation

jl-wynen
Copy link
Member

Fixes #61

This turns the formatter from scipp/copier_template#124 into a sphinx extension. I am going to add code to use it to the copier template.

Copy link
Member

@SimonHeybrock SimonHeybrock left a comment

Choose a reason for hiding this comment

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

Nice 👍

`sphinx-autodoc-typehints <https://github.com/tox-dev/sphinx-autodoc-typehints>`_
implements formatting for typehints but often results in hard-to-read annotations
for :class:`typing.NewType`.
As those are ubiquitous when using Sciline, this extension improves formatting.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
As those are ubiquitous when using Sciline, this extension improves formatting.
As those may be ubiquitous when using Sciline, this extension improves formatting.

NewType might just be how we currently use it.

Comment on lines 39 to 42
in internal modules. ``sciline.sphinxext.domain_types`` defines some default
aliases for Scipp, e.g., ``{'scipp._scipp.core.DataArray': 'scipp.DataArray', ...}``
which means that occurrences of ``scipp._scipp.core.DataArray`` are rendered as
``scipp.DataArray``. Aliases defined in ``sciline_domain_types_extra_aliases``
Copy link
Member

Choose a reason for hiding this comment

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

Sciline is independent of Scipp. Should we keep this out of the project?

Copy link
Member Author

Choose a reason for hiding this comment

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

Then we have to put it into the template to duplicate it across repos. Shouldn't be a problem, but this here seemed more convenient for our target audience.

Copy link
Member

Choose a reason for hiding this comment

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

Is there a problem with putting it into the template? If not, I would suggest to move this there.

@jl-wynen jl-wynen merged commit dbe328c into main Jan 25, 2024
5 checks passed
@jl-wynen jl-wynen deleted the typehints-extension branch January 25, 2024 11:01
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.

Documentation with links to type aliases?
2 participants