Skip to content

Conversation

vincentmacri
Copy link
Member

@vincentmacri vincentmacri commented Sep 2, 2025

Closes #40649.

I added AUTHORS in src/sage/misc/sagedoc_conf.py based on the previous copyright header. From the git history other people have touched this file, but it mostly looked like refactoring/moving code around. If I missed anyone who should be listed in the AUTHORS I'm happy to add them.

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation and checked the documentation preview.

@vincentmacri vincentmacri changed the title Docstring alias handling for function and methods Handle aliased functions and methods in generated documentation Sep 2, 2025
@vincentmacri vincentmacri marked this pull request as ready for review September 2, 2025 02:26
Copy link

github-actions bot commented Sep 2, 2025

Documentation preview for this PR (built with commit 36f57f3; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@user202729
Copy link
Contributor

user202729 commented Sep 2, 2025

looks somewhat reasonable. Is there any plan to do similar thing in __doc__ outside HTML build (e.g. in IPython-based Sage command-line ? feature)?

@kwankyu
Copy link
Collaborator

kwankyu commented Sep 2, 2025

looks somewhat reasonable. Is there any plan to do similar thing in __doc__ outside HTML build (e.g. in IPython-based Sage command-line ? feature)?

Yes. For an alias, the original docstring appears on command-line. It would be nice that that is preceded by something like "This is an alias of ...".

src/sage/misc/sphinxify.py seems responsible for the feature.

@vincentmacri
Copy link
Member Author

Is there any plan to do similar thing in __doc__ outside HTML build (e.g. in IPython-based Sage command-line ? feature)?

I wasn't planning to but I guess we could, more thoughts below.

For an alias, the original docstring appears on command-line. It would be nice that that is preceded by something like "This is an alias of ...".

The point of this PR was to remove unnecessary entries from the HTML docs. I think including the docstring for aliases on the command line is fine. Definitely don't want to remove it like this code does for the HTML docs. We could add something at the start of the docstring like you say. Do you think users might be confused by both sqrt and square_root methods existing such that explaining that one is an alias on the command line would be useful?

@kwankyu
Copy link
Collaborator

kwankyu commented Sep 2, 2025

I wasn't planning to but I guess we could, ...

OK. This PR is a great improvement as it is.

.... Do you think users might be confused by both sqrt and square_root methods existing such that explaining that one is an alias on the command line would be useful?

The docstring is for sqrt. So If you do square_root?, then the docstring, especially the examples, do not talk about sqrt_root. I think this is a bit confusing. Other examples may be more confusing.

@vincentmacri
Copy link
Member Author

I wasn't planning to but I guess we could, ...

OK. This PR is a great improvement as it is.

.... Do you think users might be confused by both sqrt and square_root methods existing such that explaining that one is an alias on the command line would be useful?

The docstring is for sqrt. So If you do square_root?, then the docstring, especially the examples, do not talk about sqrt_root. I think this is a bit confusing. Other examples may be more confusing.

I can open a new issue for handling docstrings of aliases on the command line then.

Anything else?

Copy link
Contributor

@tobiasdiez tobiasdiez left a comment

Choose a reason for hiding this comment

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

LGTM, this is a nice improvement!

Since this is a general change that other projects could also benefit from, could you please upstream your changes to the autodoc project? (Then once we rebase to the new autodoc extension that also handles alias for functions, we could remove the changes here in our doc conf)

@vincentmacri
Copy link
Member Author

Thanks for the reviews @user202729 @kwankyu @tobiasdiez!

Since this is a general change that other projects could also benefit from, could you please upstream your changes to the autodoc project? (Then once we rebase to the new autodoc extension that also handles alias for functions, we could remove the changes here in our doc conf)

I'll put it on my TODO list but not sure when I'll find the time. I have some larger mathematics PRs in the works (some work on function fields, and separate work on public-key crypto) for the next couple months that are higher priority for me.

Before upstreaming it might be good to collect the class aliasing code (which we already have in our sage_autodoc extension, which is supposed to be removed eventually per #30893) and the function/method aliasing code in one place and then upstream them together. We probably want to run it on our own repo for a while too to make sure there are no issues with this. The changes.html files indicates this PR changes over 300 documentation pages so it's very possible I missed some weird edge case in this (I'm not super worried about this though since no warnings were emitted by Sphinx, which would happen if I had an invalid link in the alias for example). Would upstreaming this be part of what is needed to eventually remove sage_autodoc, or would upstreaming this just simplify our configuration but have no effect on sage_autodoc?

@tobiasdiez
Copy link
Contributor

Would upstreaming this be part of what is needed to eventually remove sage_autodoc, or would upstreaming this just simplify our configuration but have no effect on sage_autodoc?

Perhaps it's not strictly necessary, but if there are less differences between the upstream autodoc and our custom version then a replacement of the latter becomes more feasible.

vbraun pushed a commit to vbraun/sage that referenced this pull request Sep 4, 2025
sagemathgh-40753: Handle aliased functions and methods in generated documentation
    
Closes sagemath#40649.

I added AUTHORS in `src/sage/misc/sagedoc_conf.py` based on the previous
copyright header. From the git history other people have touched this
file, but it mostly looked like refactoring/moving code around. If I
missed anyone who should be listed in the AUTHORS I'm happy to add them.

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [x] I have updated the documentation and checked the documentation
preview.
    
URL: sagemath#40753
Reported by: Vincent Macri
Reviewer(s): Kwankyu Lee, Tobias Diez, Vincent Macri
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove redundant documentation about aliased functions from HTML docs
4 participants