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

Fix intersphinx for the cosmology package #350

Closed
paddyroddy opened this issue Oct 10, 2024 · 6 comments
Closed

Fix intersphinx for the cosmology package #350

paddyroddy opened this issue Oct 10, 2024 · 6 comments
Assignees
Labels
bug Something isn't working documentation Improvements or additions to documentation help wanted Extra attention is needed question Further information is requested

Comments

@paddyroddy
Copy link
Member

What is the Topic of Your Question

Documentation

Add Your Question Below

Following #347 we now have static types rendering in the documentation. However, as can be seen in the screenshot, npt.NDArray isn't highlighted. Anyone know if it's possible?

Image

@paddyroddy paddyroddy added bug Something isn't working documentation Improvements or additions to documentation question Further information is requested labels Oct 10, 2024
@Saransh-cpp
Copy link
Member

intersphinx does this, but we will need to import numpy.typing in every file without the TYPE_CHECKING condition (sphinx-doc/sphinx#10746), or figure out a way to do that on the fly by writing a custom extension (see https://github.com/pybamm-team/PyBaMM/tree/develop/docs/sphinxext for instance).

@Saransh-cpp
Copy link
Member

Given the files have a "non-conditional" import of numpy.typing, some of the docs render (ugly imo) cross-references to numpy types -

image

Should we should stick to one thing? Either import numpy.typing without a condition or with. If we want to add the condition, we can move the types declared in this file to a utility file (_typeutils.py or something).

Originally posted by @Saransh-cpp in #347 (comment)

paddyroddy added a commit that referenced this issue Oct 14, 2024
Closes #346. Working towards #188.

The main changes here have been:
- Remove all types in the docstrings in favour of proper static typing
(being added in #308)
- Switch from `numpydoc` to `sphinx.ext.napolean` due to
numpy/numpydoc#196

Have had to change the references to `[1]` rather than `[1]_` due to
this bug, sphinx-doc/sphinx#9689. Hopefully
this can be fixed in the future.

Example:
`glass.lensing.from_convergence`
<img width="781" alt="image"
src="https://github.com/user-attachments/assets/9b6fd087-686a-4a5c-a77a-f8a3ec9fc3e2">

Raised #350, #351.
paddyroddy added a commit that referenced this issue Oct 14, 2024
Fixes #355, simplifying #308, xref #350. Change all import of
`typing`/`collections.abc` to `import <x>` rather than `from <x> import`
syntax. This is useful because it is straightforward to see at a glance
where the `import` is coming from. Further, we have both
`numpy.random.Generator` and `collections.abc.Generator` in use - so it
separates them. I've also moved everything out of the `TYPE_CHECKING`
block, except from where `ruff` says we should.
@paddyroddy paddyroddy self-assigned this Oct 24, 2024
@paddyroddy
Copy link
Member Author

So looking into this, it has nothing to do with TYPE_CHECKING. It's look alright as of #381. However, I'm trying to work out how to get it working for the cosmology package, which doesn't seem to work out the box.

@paddyroddy paddyroddy changed the title Is it possible to get hyperlinks to non in-built types? Fix intersphinx for the cosmology package Oct 29, 2024
@paddyroddy paddyroddy changed the title Fix intersphinx for the cosmology package Fix intersphinx for the cosmology package Oct 29, 2024
@paddyroddy paddyroddy added the help wanted Extra attention is needed label Oct 29, 2024
@paddyroddy
Copy link
Member Author

paddyroddy commented Oct 29, 2024

So looks like everything is working except https://cosmology.readthedocs.io/en/stable. I tried both

intersphinx_mapping = {
    "cosmology": ("https://cosmology.readthedocs.io/en/stable", None),
    "numpy": ("https://numpy.org/doc/stable", None),
    "python": ("https://docs.python.org/3", None),
}

and

intersphinx_mapping = {
    "cosmology": (
        "https://cosmology.readthedocs.io/en/stable",
        "https://cosmology.readthedocs.io/en/stable/objects.inv",
    ),
    "numpy": ("https://numpy.org/doc/stable", None),
    "python": ("https://docs.python.org/3", None),
}

any ideas? Neither worked. Won't matter if #124 is tackled first.

@Saransh-cpp
Copy link
Member

I am tempted to move away from cosmology as soon as possible. The move will also help us with Array API stuff. We can maybe discuss this in the Monday meeting.

@paddyroddy
Copy link
Member Author

I'm going to close this. We can come back to it if we have issues with #124.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation help wanted Extra attention is needed question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants