Add external packages to `intersphinx_mapping` in `conf.py` #4290

nzw0301 · 2022-12-28T06:02:06Z

Motivation

Investigation for the category c:

c. Reference to API's in other libraries, such as numpy or torch

Description of the changes

as described in the issue

c. we might want to agree on some policy what libraries to include and test to make sure it works. C.f. https://github.com/optuna/optuna/blob/master/docs/source/conf.py and https://docs.readthedocs.io/en/stable/guides/intersphinx.html

conf.py's intersphinx_mapping enables docs to link the external packages.

nzw0301 · 2022-12-28T06:05:07Z

When we use alias to import packages, such as pandas as pd, the link doesn't work as we expect, ref: sphinx-doc/sphinx#10151. For example, pd.DataFrame on optuna.study.study.Study.trials_dataframe and Plotly's plotly.graph_objs._figure.Figure (ref: https://stackoverflow.com/questions/73528299/link-to-plotly-graph-objects-figure-with-intersphinx).

Sorry, the explanation was probably wrong since np.* works well.

codecov-commenter · 2022-12-28T06:36:46Z

Codecov Report

Merging #4290 (33f4d88) into master (e8a010b) will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #4290      +/-   ##
==========================================
- Coverage   89.77%   89.75%   -0.02%     
==========================================
  Files         170      170              
  Lines       13265    13265              
==========================================
- Hits        11908    11906       -2     
- Misses       1357     1359       +2

Impacted Files	Coverage Δ
optuna/integration/sklearn.py	`96.75% <ø> (ø)`
optuna/study/study.py	`94.18% <0.00%> (-0.78%)`	⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

HideakiImamura · 2023-01-04T01:48:37Z

@nzw0301 Thanks for the PR! Is this PR ready for review or needed to discuss something?

HideakiImamura · 2023-01-04T05:07:22Z

@toshihikoyanase @contramundum53 Could you review this PR?

nzw0301 · 2023-01-04T06:08:38Z

docs/source/conf.py

+    "sklearn": ("https://scikit-learn.org/stable", None),
+    "torch": ("https://pytorch.org/docs/stable", None),
+    "pandas": ("https://pandas.pydata.org/docs", None),
+    "plotly": ("https://plotly.com/python-api-reference", None),


[Comments to reveiwers]: This might be unnecessary because the links to plotly's API are broken due to https://stackoverflow.com/questions/73528299/link-to-plotly-graph-objects-figure-with-intersphinx. Possibly future intersphinx or Plotly fixes this issue, so we can also leave this plotly part as in this PR (Pandas as well).

nzw0301 · 2023-01-04T06:21:11Z

Let le leave my comments on #4213 (comment):

c. we might want to agree on some policy what libraries to include and test to make sure it works

Point 1: Policy what libraries

This PR only includes a part of libraries used in the document section in setup.py. The added libraries to intersphinx_mapping in docs/conf.py are used at least once in the Optuna docs.

Point 2: Test to make sure it works

I just manually checked the warning messages by nitpicky.

contramundum53 · 2023-01-05T09:21:01Z

The link seems to be working. Thanks for the PR!

github-actions · 2023-01-12T23:05:44Z

This pull request has not seen any recent activity.

toshihikoyanase

I'm sorry for the delayed response. The change seems reasonable to me, but let me check again after the release of v3.1.

github-actions · 2023-01-24T23:05:18Z

This pull request has not seen any recent activity.

HideakiImamura · 2023-01-30T01:39:59Z

@toshihikoyanase A friendly reminder :)

toshihikoyanase · 2023-01-30T01:41:20Z

Thanks. I'll check it.

toshihikoyanase

I'm sorry for the delayed response. The change looks great to me.
Maybe we have some follow-up tasks, but I don't think we should fix them in this PR.

toshihikoyanase · 2023-02-03T01:17:51Z

docs/source/conf.py

+    "matplotlib": ("https://matplotlib.org/stable", None),
+    "numpy": ("https://numpy.org/doc/stable", None),
+    "scipy": ("https://docs.scipy.org/doc/scipy", None),
+    "sklearn": ("https://scikit-learn.org/stable", None),


toshihikoyanase · 2023-02-03T01:18:32Z

docs/source/conf.py

-intersphinx_mapping = {"python": ("https://docs.python.org/3", None)}
+intersphinx_mapping = {
+    "python": ("https://docs.python.org/3", None),
+    "distributed": ("https://distributed.dask.org/en/stable", None),


toshihikoyanase · 2023-02-03T01:19:53Z

docs/source/conf.py

+intersphinx_mapping = {
+    "python": ("https://docs.python.org/3", None),
+    "distributed": ("https://distributed.dask.org/en/stable", None),
+    "lightgbm": ("https://lightgbm.readthedocs.io/en/latest", None),


toshihikoyanase · 2023-02-03T01:20:42Z

docs/source/conf.py

+    "python": ("https://docs.python.org/3", None),
+    "distributed": ("https://distributed.dask.org/en/stable", None),
+    "lightgbm": ("https://lightgbm.readthedocs.io/en/latest", None),
+    "matplotlib": ("https://matplotlib.org/stable", None),


toshihikoyanase · 2023-02-03T01:24:36Z

docs/source/conf.py

+    "distributed": ("https://distributed.dask.org/en/stable", None),
+    "lightgbm": ("https://lightgbm.readthedocs.io/en/latest", None),
+    "matplotlib": ("https://matplotlib.org/stable", None),
+    "numpy": ("https://numpy.org/doc/stable", None),


toshihikoyanase · 2023-02-03T01:26:03Z

docs/source/conf.py

+    "lightgbm": ("https://lightgbm.readthedocs.io/en/latest", None),
+    "matplotlib": ("https://matplotlib.org/stable", None),
+    "numpy": ("https://numpy.org/doc/stable", None),
+    "scipy": ("https://docs.scipy.org/doc/scipy", None),


toshihikoyanase · 2023-02-03T01:29:18Z

docs/source/conf.py

+    "numpy": ("https://numpy.org/doc/stable", None),
+    "scipy": ("https://docs.scipy.org/doc/scipy", None),
+    "sklearn": ("https://scikit-learn.org/stable", None),
+    "torch": ("https://pytorch.org/docs/stable", None),


[Notes] torch.distributed.ProcessGroup was not linked to the pytorch docs. The URL seems to be correct, and we may need to update the docstring/type hints in a follow-up PR.

toshihikoyanase · 2023-02-03T01:31:11Z

docs/source/conf.py

+    "scipy": ("https://docs.scipy.org/doc/scipy", None),
+    "sklearn": ("https://scikit-learn.org/stable", None),
+    "torch": ("https://pytorch.org/docs/stable", None),
+    "pandas": ("https://pandas.pydata.org/docs", None),


[Notes] The URL seems to be correct, but the pd.Dataframe is not linked to the pandas docs. Maybe we need to update the type hint in a follow-up PR.

Add external packages to intersphinx_mapping in conf.py

519e3b3

nzw0301 mentioned this pull request Dec 28, 2022

Documentation warnings with Sphinx nitpicky #4213

Closed

nzw0301 added the document Documentation related. label Dec 28, 2022

Fix path to sklearn.base.BaseEstimator for reference

33f4d88

nzw0301 marked this pull request as ready for review January 4, 2023 03:33

HideakiImamura assigned toshihikoyanase and contramundum53 Jan 4, 2023

nzw0301 commented Jan 4, 2023

View reviewed changes

contramundum53 approved these changes Jan 5, 2023

View reviewed changes

github-actions bot added the stale Exempt from stale bot labeling. label Jan 12, 2023

toshihikoyanase reviewed Jan 17, 2023

View reviewed changes

github-actions bot removed the stale Exempt from stale bot labeling. label Jan 17, 2023

github-actions bot added the stale Exempt from stale bot labeling. label Jan 24, 2023

github-actions bot removed the stale Exempt from stale bot labeling. label Jan 30, 2023

toshihikoyanase approved these changes Feb 3, 2023

View reviewed changes

toshihikoyanase added this to the v3.2.0 milestone Feb 3, 2023

toshihikoyanase merged commit 1bb68cd into optuna:master Feb 3, 2023

nzw0301 deleted the test-add-external-packages-link branch February 3, 2023 09:21

This was referenced Sep 11, 2023

Fix the documentation warnings with Sphinx nitpicky: torch.distributed.ProcessGroup is not linked to the pytorch document #4916

Open

Fix the documentation warnings with Sphinx nitpicky: pd.Dataframe is not linked to the pandas document #4917

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add external packages to `intersphinx_mapping` in `conf.py` #4290

Add external packages to `intersphinx_mapping` in `conf.py` #4290

nzw0301 commented Dec 28, 2022

nzw0301 commented Dec 28, 2022 •

edited

codecov-commenter commented Dec 28, 2022 •

edited

HideakiImamura commented Jan 4, 2023

HideakiImamura commented Jan 4, 2023

nzw0301 Jan 4, 2023 •

edited

nzw0301 commented Jan 4, 2023

contramundum53 commented Jan 5, 2023

github-actions bot commented Jan 12, 2023

toshihikoyanase left a comment

github-actions bot commented Jan 24, 2023

HideakiImamura commented Jan 30, 2023

toshihikoyanase commented Jan 30, 2023

toshihikoyanase left a comment

toshihikoyanase Feb 3, 2023

toshihikoyanase Feb 3, 2023

toshihikoyanase Feb 3, 2023

toshihikoyanase Feb 3, 2023

toshihikoyanase Feb 3, 2023

toshihikoyanase Feb 3, 2023

toshihikoyanase Feb 3, 2023

toshihikoyanase Feb 3, 2023

Add external packages to intersphinx_mapping in conf.py #4290

Add external packages to intersphinx_mapping in conf.py #4290

Conversation

nzw0301 commented Dec 28, 2022

Motivation

Description of the changes

nzw0301 commented Dec 28, 2022 • edited

codecov-commenter commented Dec 28, 2022 • edited

Codecov Report

HideakiImamura commented Jan 4, 2023

HideakiImamura commented Jan 4, 2023

nzw0301 Jan 4, 2023 • edited

Choose a reason for hiding this comment

nzw0301 commented Jan 4, 2023

Point 1: Policy what libraries

Point 2: Test to make sure it works

contramundum53 commented Jan 5, 2023

github-actions bot commented Jan 12, 2023

toshihikoyanase left a comment

Choose a reason for hiding this comment

github-actions bot commented Jan 24, 2023

HideakiImamura commented Jan 30, 2023

toshihikoyanase commented Jan 30, 2023

toshihikoyanase left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Add external packages to `intersphinx_mapping` in `conf.py` #4290

Add external packages to `intersphinx_mapping` in `conf.py` #4290

nzw0301 commented Dec 28, 2022 •

edited

codecov-commenter commented Dec 28, 2022 •

edited

nzw0301 Jan 4, 2023 •

edited