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

Rebase sage_autodoc to sphinx 5.3.0 #34730

Closed
kwankyu opened this issue Nov 8, 2022 · 25 comments
Closed

Rebase sage_autodoc to sphinx 5.3.0 #34730

kwankyu opened this issue Nov 8, 2022 · 25 comments

Comments

@kwankyu
Copy link
Collaborator

kwankyu commented Nov 8, 2022

We rebase sage_autodoc to Sphinx 5.3.0.

This is a step toward eventual removal of sage_autodoc in #30893, a customization of Sphinx autodoc extension for Sage objects.

Other related tickets are #27578. #30884, #31309, in this regard.

CC: @kiwifb

Component: documentation

Author: Kwankyu Lee

Branch/Commit: aa74d4b

Reviewer: Tobias Diez

Issue created by migration from https://trac.sagemath.org/ticket/34730

@kwankyu kwankyu added this to the sage-9.8 milestone Nov 8, 2022
@kwankyu
Copy link
Collaborator Author

kwankyu commented Nov 8, 2022

@kwankyu
Copy link
Collaborator Author

kwankyu commented Nov 8, 2022

Author: Kwankyu Lee

@kwankyu
Copy link
Collaborator Author

kwankyu commented Nov 8, 2022

Commit: 94a9718

@kwankyu

This comment has been minimized.

@kwankyu
Copy link
Collaborator Author

kwankyu commented Nov 8, 2022

comment:3

Setting "needs review" to build the documentation for further check.

@kwankyu

This comment has been minimized.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 8, 2022

Changed commit from 94a9718 to 3e917a4

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 8, 2022

Branch pushed to git repo; I updated commit sha1. New commits:

3e917a4Remove spurious changes

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 9, 2022

Changed commit from 3e917a4 to aa74d4b

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 9, 2022

Branch pushed to git repo; I updated commit sha1. New commits:

aa74d4bMore edits

@kwankyu

This comment has been minimized.

@kwankyu

This comment has been minimized.

@kwankyu

This comment has been minimized.

@kiwifb
Copy link
Member

kiwifb commented Nov 9, 2022

comment:13

Do you have a ticket for sphinx 5.3.0 or is it backward compatible?

@kwankyu
Copy link
Collaborator Author

kwankyu commented Nov 9, 2022

comment:14

Replying to François Bissey:

Do you have a ticket for sphinx 5.3.0 or is it backward compatible?

There is no difference between autodocs of sphinx 5.2.3 and 5.3.0.

@kwankyu
Copy link
Collaborator Author

kwankyu commented Nov 9, 2022

comment:15

Moreover, as our old sage_autodoc (that is based on sphinx 1.8.x) still works with sphinx 5.2.3, autodoc extension seems pretty independent from the version of sphinx.

@kiwifb
Copy link
Member

kiwifb commented Nov 9, 2022

comment:16

I am fairly it has been touched here and there. But because of the ticket title I was wondering if we were doing something specific for sphinx 5.3.0. I have not checked we are compatible with sphinx 5.3.0 for example (and presumably if Antonio Rojas tested it, he didn't find/report any issues).

@kwankyu
Copy link
Collaborator Author

kwankyu commented Nov 10, 2022

comment:17

Replying to François Bissey:

I am fairly it has been touched here and there. But because of the ticket title I was wondering if we were doing something specific for sphinx 5.3.0.

I just used the latest from sphinx 5.x series.

@kwankyu
Copy link
Collaborator Author

kwankyu commented Nov 10, 2022

comment:18

Replying to François Bissey:

I have not checked we are compatible with sphinx 5.3.0 for example (and presumably if Antonio Rojas tested it, he didn't find/report any issues).

When I did the rebase for sphinx 1.8.x, there were many regressions, which Jeroen Demeyer detected and reported to me. If there are none (or few) this time, that is because of the efforts made then. This time it is not that difficult. Next time, hopefully it would be even easier :)

@tobiasdiez
Copy link
Contributor

comment:19

LGTM.

Since you are now familiar with the code, can you maybe list the major changes of sage's autodoc in #30893 with a proposal for how to remove the modification.

@tobiasdiez

This comment has been minimized.

@tobiasdiez
Copy link
Contributor

Reviewer: Tobias Diez

@kwankyu
Copy link
Collaborator Author

kwankyu commented Nov 10, 2022

comment:20

Replying to Tobias Diez:

LGTM.

Thanks.

Since you are now familiar with the code,

Well.., it was basically trial-and-errors with bits of understanding here and there :)

can you maybe list the major changes of sage's autodoc in #30893 with a proposal for how to remove the modification.

I marked in the code where modifications are made. It is easy to list them:

  1. skipping lazy imports to avoid deprecation warnings
  2. providing argspecs for decorators (sage_wraps)
  3. dealing with nested classes
  4. dealing with cached function(methods) callers
  5. dealing with classcall metaclass
  6. dealing with class aliases
  7. using argspecs instead of signature

but sorry no proposal for how to remove them. All looks difficult. I am not sure if the aim of eventual removal of sage_autodoc is really achievable.

After #26254 merged, switching to signature seems the most urgent thing.

@tobiasdiez
Copy link
Contributor

comment:21

Thanks for the summary, I've added it to #30893.

@vbraun
Copy link
Member

vbraun commented Nov 15, 2022

Changed branch from u/klee/rebase_sage_autodoc_to_sphinx_5_3_0 to aa74d4b

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

No branches or pull requests

4 participants