Opened 3 years ago

Closed 2 years ago

#22604 closed enhancement (fixed)

autodoc unforking again

Reported by: jdemeyer Owned by:
Priority: major Milestone: sage-8.2
Component: build Keywords:
Cc: hivert, embray Merged in:
Authors: Jeroen Demeyer Reviewers: Erik Bray
Report Upstream: N/A Work issues:
Branch: 14b02b0 (Commits) Commit: 14b02b01c572840e1c7958a80a397af840f7b1ed
Dependencies: Stopgaps:

Description

Move some changes from upstream Sphinx autodoc.py to sage_autodoc.py

Change History (14)

comment:1 Changed 3 years ago by jdemeyer

  • Dependencies set to #22601

comment:2 Changed 3 years ago by jdemeyer

  • Branch set to u/jdemeyer/ticket/22604

comment:3 Changed 3 years ago by jdemeyer

  • Commit set to 130be6d92070befcf662862c1900156656b5f1ec
  • Status changed from new to needs_review

This isn't perfect but it's a step in the right direction.


New commits:

879c401Upgrade to Sphinx 1.5.3
78e6718Docbuild fixes for Sphinx 1.5.x
ced4920Allow running Sphinx without SSL
3a4e3a1Upgrade docutils
130be6dMove upstream changes to autodoc into Sage

comment:4 Changed 2 years ago by jdemeyer

  • Cc embray added
  • Dependencies #22601 deleted
  • Milestone changed from sage-7.6 to sage-8.2

comment:5 follow-up: Changed 2 years ago by embray

I'm not exactly clear on what this is doing. Is sage_autodoc a copy of the autodoc module from Sphinx with some patches applied? Is the point here just to re-apply those patches on top of a newer copy of the module?

(I assume the goal is to eventually do away with the copy entirely...?)

comment:6 in reply to: ↑ 5 Changed 2 years ago by jdemeyer

Replying to embray:

Is sage_autodoc a copy of the autodoc module from Sphinx with some patches applied?

Yes.

Is the point here just to re-apply those patches on top of a newer copy of the module?

No. The point is to reduce the patching, i.e. reduce the diff between the real autodoc and the Sage autodoc.

Ideally, we should eventually reduce the diff to zero (by fixing Sage and/or Sphinx).

comment:7 Changed 2 years ago by embray

Well reduce, sure, but it seems like some of both. That is, you started with the new upstream version, re-applied any patches from Sage, sans patches that are no longer necessary.

comment:8 follow-ups: Changed 2 years ago by embray

It might be nice if there were a comment in the module specifying exactly what version of the upstream code this is based on (e.g. with a link to GitHub at the correct revision). That way it would be easy enough to compare to the original to see what the differences are.

Last edited 2 years ago by embray (previous) (diff)

comment:9 in reply to: ↑ 8 Changed 2 years ago by jdemeyer

Replying to embray:

It might be nice if there were a comment in the module specifying exactly what version of the upstream code this is based on

It all started a very long time ago as a fork of the Sphinx autodoc. Many changes have been made, some upstream changes were merged. It's not meaningful to say that it is a fork of a single upstream commit, it's much messier than that.

comment:10 in reply to: ↑ 8 Changed 2 years ago by jdemeyer

Replying to embray:

compare to the original to see what the differences are.

When I created this ticket, Sage was using Sphinx 1.5.3 so I guess it's best to compare against that: https://github.com/sphinx-doc/sphinx/blob/v1.5.3/sphinx/ext/autodoc.py

comment:11 Changed 2 years ago by embray

  • Reviewers set to Erik Bray

What I mean is to just add such a link in the comments in the source, sort of like I did in the comment near the top of https://git.sagemath.org/sage.git/tree/src/sage/cpython/_py2_random.py?id=3ee11e056d80c4a9163e5a8d1a8c92e12c34f25b

You can set positive review once that's done so we can move forward on this.

comment:12 Changed 2 years ago by git

  • Commit changed from 130be6d92070befcf662862c1900156656b5f1ec to 14b02b01c572840e1c7958a80a397af840f7b1ed

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

14b02b0Move upstream changes to autodoc into Sage

comment:13 Changed 2 years ago by jdemeyer

  • Status changed from needs_review to positive_review

comment:14 Changed 2 years ago by vbraun

  • Branch changed from u/jdemeyer/ticket/22604 to 14b02b01c572840e1c7958a80a397af840f7b1ed
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.