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

Document function initialization parameters #24398

Closed
rwst opened this issue Dec 18, 2017 · 29 comments
Closed

Document function initialization parameters #24398

rwst opened this issue Dec 18, 2017 · 29 comments

Comments

@rwst
Copy link
Contributor

rwst commented Dec 18, 2017

The classes in symbolic/function.pyx need better docs, esp. the parameters.

Component: documentation

Author: Ralf Stephan

Branch/Commit: 469d0e2

Reviewer: Markus Wageringel, Dima Pasechnik

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

@rwst rwst added this to the sage-8.2 milestone Dec 18, 2017
@rwst

This comment has been minimized.

@rwst
Copy link
Contributor Author

rwst commented Mar 1, 2018

@rwst

This comment has been minimized.

@rwst
Copy link
Contributor Author

rwst commented Mar 1, 2018

New commits:

d8ae5e224398: Document function initialization parameters

@rwst
Copy link
Contributor Author

rwst commented Mar 1, 2018

Author: Ralf Stephan

@rwst
Copy link
Contributor Author

rwst commented Mar 1, 2018

Commit: d8ae5e2

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 4, 2018

Changed commit from d8ae5e2 to 3cd27de

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 4, 2018

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

3cd27de24398: remove include of other documents

@bryangingechen
Copy link
Mannequin

bryangingechen mannequin commented Jul 28, 2018

comment:6

This documentation is really helpful!

Suggestions:

  • In src/doc/en/reference/functions/index.rst, add a link to src/sage/symbolic/function.pyx
  • Presumably you meant to remove the fragment at the end of this line
+ * :class:`BuiltinFunction`: the code of these functions is written in Python; many special functions are of this type, see :doc

This may be out-of-scope for this ticket but it's so trivial that I must point it out:

  • There is a formatting error in the docstring for sage.symbolic.function.get_sfunction_from_serial, there should be double backticks around sage.symbolic.function.sfunction_serial_dict

@dimpase
Copy link
Member

dimpase commented Dec 11, 2018

comment:7

I'm willing to give this a positive review once comment 6 is addressed.

@dimpase dimpase modified the milestones: sage-8.2, sage-8.5 Dec 11, 2018
@jdemeyer
Copy link
Contributor

jdemeyer commented Jan 6, 2019

comment:8

Two quick comments:

  1. Please find extensive developer documentation for creating new functions in the symbolic calculus module. is not really helpful without a hyperlink to the symbolic calculus module.

  2. Can you limit line lengths? Some lines are rather long.

@egourgoulhon
Copy link
Member

@egourgoulhon
Copy link
Member

New commits:

8320200Merge branch 'u/rws/document_function_initialization_parameters' of git://trac.sagemath.org/sage into Sage 9.0.beta7
e1db2f9Minor fixes in the documentation of symbolic functions (trac 24398)

@egourgoulhon
Copy link
Member

Changed commit from 3cd27de to e1db2f9

@dimpase dimpase modified the milestones: sage-8.5, sage-9.0 Dec 1, 2019
@egourgoulhon
Copy link
Member

comment:11

The above commit implements the corrections suggested in comment:6 and comment:8.
It would be nice to have this in Sage 9.0...

@egourgoulhon egourgoulhon modified the milestones: sage-9.0, sage-8.5 Dec 1, 2019
@mwageringel
Copy link
Contributor

comment:13

The documentation does not build.

@dimpase
Copy link
Member

dimpase commented Dec 2, 2019

comment:14

basically, one cannot use :doc: to refer to index, one needs something like
list <../../../functions/index.html> etc. I'll push a fix soon.

@egourgoulhon
Copy link
Member

comment:15

Replying to @dimpase:

basically, one cannot use :doc: to refer to index,

Indeed. It was working in my case probably because the referred document had been already created by a previous make doc.

one needs something like
list <../../../functions/index.html> etc. I'll push a fix soon.

Another solution would be to add a tag in src/doc/en/reference/functions/index.rst, special-functions say, and refer to it as :ref:`list `.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 2, 2019

Changed commit from e1db2f9 to 469d0e2

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 2, 2019

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

469d0e2Fix hyperlinks in the documentation of symbolic functions (trac 24398)

@egourgoulhon
Copy link
Member

comment:17

Replying to @egourgoulhon:

Replying to @dimpase:

one needs something like
list <../../../functions/index.html> etc. I'll push a fix soon.

Another solution would be to add a tag in src/doc/en/reference/functions/index.rst, special-functions say, and refer to it as :ref:`list `.

I've implemented the latter solution in the last commit. Do you agree it is more robust? in particular when generating the pdf documentation (I guess something like ../../../functions/index.html would fail then).

@dimpase
Copy link
Member

dimpase commented Dec 2, 2019

comment:18

This seems to work, thanks.
I'd rather use tag names like

+.. _calculus-index:

than

+.. _symbolic-calculus:

to make it more clear where it points to.

We have quite a number of links like <../../../foo/index.html> in src/sage, perhaps all such links should be replaces the way you propose?

I also wonder whether we should rather be using https://www.sphinx-doc.org/en/master/usage/extensions/autosectionlabel.html
which apparently allows using headings as labels, and so explicit tags won't be needed
(I must say I don't know how to enable it in our oh so transparent sphinx configs...)

@dimpase
Copy link
Member

dimpase commented Dec 2, 2019

comment:19

it appears that autosectionlabel doesn't fly well with Sage (one gets name clashes all over the place, from autogenerated rst files). So unless this is reworked somehow we can't use it.

@dimpase
Copy link
Member

dimpase commented Dec 2, 2019

Reviewer: Markus Wageringel, Dima Pasechnik

@dimpase
Copy link
Member

dimpase commented Dec 2, 2019

comment:20

OK, let's get this in.

@mwageringel
Copy link
Contributor

comment:21

Replying to @dimpase:

We have quite a number of links like <../../../foo/index.html> in src/sage, perhaps all such links should be replaces the way you propose?

As far as I know, the tags only work within a single sphinx document, but we have many documents which is a reason for many of the relative links to exist.

@egourgoulhon
Copy link
Member

comment:22

Replying to @dimpase:

it appears that autosectionlabel doesn't fly well with Sage (one gets name clashes all over the place, from autogenerated rst files). So unless this is reworked somehow we can't use it.

Thanks for having given it a try. This certainly would have been a better solution...

@egourgoulhon
Copy link
Member

comment:23

Replying to @mwageringel:

Replying to @dimpase:

We have quite a number of links like <../../../foo/index.html> in src/sage, perhaps all such links should be replaces the way you propose?

As far as I know, the tags only work within a single sphinx document, but we have many documents which is a reason for many of the relative links to exist.

Not always: the following links are inside the same document (the reference manual):

rings/big_oh.py:    - `asymptotic expansions <../../../asymptotic/index.html>`_
rings/big_oh.py:    - `p-adic numbers <../../../padics/index.html>`_
rings/big_oh.py:    - `power series <../../../power_series/index.html>`_
rings/big_oh.py:    - `polynomials <../../../polynomial_rings/index.html>`_
rings/asymptotic/asymptotic_ring.py:`coercion <../../../../coercion/index.html>`_. 

@vbraun
Copy link
Member

vbraun commented Dec 4, 2019

Changed branch from public/symbolic/document_functions-24398 to 469d0e2

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

6 participants