-
-
Notifications
You must be signed in to change notification settings - Fork 435
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
New feature annotations # optional - sage.schemes sage.modular sage.libs.flint
etc.
#35728
Conversation
…ial.pbori; add # optional
src/sage/arith/misc.py
Outdated
@@ -1693,8 +1699,8 @@ def plot(self, xmin=1, xmax=50, k=1, pointsize=30, rgbcolor=(0,0,1), join=True, | |||
EXAMPLES:: | |||
|
|||
sage: from sage.arith.misc import Sigma | |||
sage: p = Sigma().plot() | |||
sage: p.ymax() | |||
sage: p = Sigma().plot() # optional - sage.plot # optional - sage.libs.pari |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not using # optional - sage.plot sage.libs.pari
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
src/sage/arith/misc.py
Outdated
@@ -82,109 +82,109 @@ def algdep(z, degree, known_bits=None, use_bits=None, known_digits=None, | |||
|
|||
EXAMPLES:: | |||
|
|||
sage: algdep(1.888888888888888, 1) | |||
sage: algdep(1.888888888888888, 1) # optional - sage.libs.pari |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand the alignment of the # optional - ...
in this file. It varies between blocks. Furthermore, some lines have 2 # optional - ...
tags.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, this was completely messed up. Fixed now.
@@ -1,3 +1,4 @@ | |||
# sage.doctest: optional - primecountpy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be so cool if all files could be like this one with only the declaration of the overall optional dependencies. Of course it's much more complicated than that in practice as some optional tags concern only a few tests in a file and are really iff some optional package (e.g., igraph) has been installed. But for dependencies that are repeated on almost all lines, it could ease our life to force the entire file to depend on these packages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, and in some cases we can achieve this by splitting out parts of files to a separate file (but probably we should do this only where it seems natural).
- For example, with @kwankyu we did this in
sage.rings.function_field
: Modularization fixes #35230 - to avoid having to mark one half of each of several files by# optional - sage.libs.singular
. - See also
sage.combinat
: More# optional
annotations #35742 (comment)
src/sage/features/sagemath.py
Outdated
EXAMPLES:: | ||
|
||
sage: from sage.features.sagemath import sage__schemes | ||
sage: sage__schemes().is_present() # optional - sage.schemes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't we have more spaces before the # optional
tag ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Done in 3c002ac; the same for the existing features in this file is taken care of in:
|
||
sage: proof.arithmetic(True) | ||
sage: with proof.WithProof('arithmetic',False): # this would hang "forever" if attempted with proof=True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless this comment is no longer relevant, it should remain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment appears in text just above:
sage/src/sage/structure/proof/proof.py
Lines 200 to 208 in 8199be3
This would hang "forever" if attempted with ``proof=True``:: | |
sage: proof.arithmetic(True) | |
sage: with proof.WithProof('arithmetic', False): # optional - sage.libs.pari | |
....: print((10^1000 + 453).is_prime()) | |
....: print(1/0) | |
Traceback (most recent call last): | |
... |
Documentation preview for this PR (built with commit 4f968a8) is ready! 🎉 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Thank you! |
Let's wait until we come to a conclusion at https://groups.google.com/g/sage-devel/c/utA0N1it0Eo how to proceed with these optional annotations. |
Tobias, as I have explained before, the adding these annotations is already documented as our practice. |
📚 Description
📝 Checklist
⌛ Dependencies