sage -t --optional='sage,!FEATURE'
Closed this issue · 31 comments
sage -t --optional='sage,!FEATURE' will disable autodetection of FEATURE.
Useful for checking sequences of # optional - FEATURE annotations in particular when FEATURE is standard in Sage, for example sage.misc.cython (#33029):
./sage -t --optional='sage,!sage.misc.cython' src/sage/misc/inherit_comparison.pyx
Also, sage -t --optional='sage,optional,!FEATURE' will remove it from the list of optional features supplied by the package list and disable auto-detection.
For example, when the optional SPKG bliss is installed, --optional='sage,optional' would expand to a list including bliss. By using --optional='sage,optional,!bliss', it can be removed.
CC: @kwankyu @orlitzky @kiwifb @seblabbe
Component: refactoring
Author: Matthias Koeppe
Branch: ba86146
Reviewer: Sebastian Oehms
Issue created by migration from https://trac.sagemath.org/ticket/33823
Description changed:
---
+++
@@ -1,4 +1,8 @@
This will disable autodetection of `FEATURE`.
-Useful for checking `# optional - FEATURE` annotations in particular when `FEATURE` is standard in Sage, for example `sage.misc.cython` (#33029).
+Useful for checking `# optional - FEATURE` annotations in particular when `FEATURE` is standard in Sage, for example `sage.misc.cython` (#33029):
+```
+./sage -t --optional='sage,!sage.misc.cython' src/sage/misc/inherit_comparison.pyx
+```
+Author: Matthias Koeppe
Description changed:
---
+++
@@ -1,4 +1,4 @@
-This will disable autodetection of `FEATURE`.
+`sage -t --optional='sage,!FEATURE'` will disable autodetection of `FEATURE`.
Useful for checking `# optional - FEATURE` annotations in particular when `FEATURE` is standard in Sage, for example `sage.misc.cython` (#33029):
@@ -6,3 +6,9 @@
./sage -t --optional='sage,!sage.misc.cython' src/sage/misc/inherit_comparison.pyx
```
+
+Also, `sage -t --optional='sage,optional,!FEATURE'` will remove it from the list of optional features supplied by the package list and disable auto-detection.
+
+For example, when the optional SPKG `bliss` is installed, `--optional='sage,optional'` would expand to a list including `bliss`. By using `--optional='sage,optional,!bliss'`, it can be removed.
+
+Branch pushed to git repo; I updated commit sha1. New commits:
d0a52ed | src/bin/sage-runtests: Update documentation of --optional |
How about respecting --disable-bliss instead?
We don't have a mechanism for that, and it does not help for the main use case of this ticket - disabling standard features such as sage.misc.cython when doctesting.
Yes, you are giving yourself some tools to be able to isolate what you are testing more. Which is interesting in and of itself. But if we completely migrate to pytest (let's be honest that will/would take years), will that feature survive the migration? Does pytest support such disabling?
Replying to @mkoeppe:
We don't have a mechanism for that
It worked for years until you added runtime detection that overrides it. Now you want to add a third layer of complexity without fixing the previous two.
I posted one possible solution recently on the mailing list and another ticket. Another (inevitable, eventually) option is to add a ./configure script to whatever modularized component of sage requires bliss, and to copy AC_ARG_ENABLE([bliss]) there. Then the "feature" that detects it from the other components becomes simply e.g. import sage.whatever.BLISS. There is already precedent for having setup.py run sh ./configure to keep that component pip-installable.
Replying to @orlitzky:
Replying to @mkoeppe:
We don't have a mechanism for that
It worked for years until you added runtime detection that overrides it.
Sage has never had a mechanism to disable the use of optional features by the Sage library.
The only thing that has changed is what the doctester tests. The present ticket gives you the tool that you need if you have problems with an optional package in your distribution packaging.
Let's keep the ticket focused. It is not meant as an invitation to continue your musings from sage-devel
Replying to @kiwifb:
Yes, you are giving yourself some tools to be able to isolate what you are testing more. Which is interesting in and of itself. But if we completely migrate to pytest (let's be honest that will/would take years), will that feature survive the migration? Does pytest support such disabling?
Conditionalizing tests on the presence of features is a key idea of the modularization, and we'll make it work with pytest too. Skipping tests based on predicates is a standard feature of all test frameworks.
#33546 implements the Sage doctest discovery in pytest. It continues to use code from sage.doctest, so I see no issue in making # optional annotations work, but I don't know the details.
Description changed:
---
+++
@@ -1,6 +1,6 @@
`sage -t --optional='sage,!FEATURE'` will disable autodetection of `FEATURE`.
-Useful for checking `# optional - FEATURE` annotations in particular when `FEATURE` is standard in Sage, for example `sage.misc.cython` (#33029):
+Useful for checking sequences of `# optional - FEATURE` annotations in particular when `FEATURE` is standard in Sage, for example `sage.misc.cython` (#33029):
```
./sage -t --optional='sage,!sage.misc.cython' src/sage/misc/inherit_comparison.pyx I think it's a good idea to have a negation possibility here. Maybe the following needs a clarification in the help-string:
In a positive list of features you don't need to type ' and maybe a lot of people use it in this way. But if you want to exclude a feature using this new functionality you must type them or use \! instead of !.
BTW: If you have installed some optional feature xyz on your system and forgot a # optional - xyz or xyz.is_present() somewhere in your change of code then this new doctest option will not bring this to your attention. That was what I hoped when I've first seen this ticket. Do you know, how this can be achieved without uninstalling the package?
Further: I really don't know the intention of the log-message Features detected for doctesting. My observation is that this messages shows nothing even though optional tests were performed. At least this is irritating. Example:
Using --optional=!database_knotinfo,sage
Features to be detected: 4ti2,benzene,bliss,buckygen,conway_polynomials,csdp,database_cremona_ellcurve,database_cremona_mini_ellcurve,database_jones_numfield,dvipng,gfan,graphviz,imagemagick,jupymake,kenzo,latte_int,lrslib,mcqd,meataxe,msolve,nauty,palp,pandoc,pdf2svg,pdftocairo,phitigra,plantri,polytopes_db,polytopes_db_4d,pynormaliz,python_igraph,rubiks,sage.combinat,sage.geometry.polyhedron,sage.graphs,sage.groups,sage.plot,sage.rings.number_field,sage.rings.padics,sage.rings.real_double,sage.symbolic,sage_numerical_backends_coin,sagemath_doc_html,sphinx,tdlib
Doctesting 1 file.
sage -t --random-seed=71318033978106427613212696911118388467 src/sage/knots/knotinfo.py
[323 tests, 5.51 s]
----------------------------------------------------------------------
All tests passed!
----------------------------------------------------------------------
Total time for all tests: 5.6 seconds
cpu time: 5.5 seconds
cumulative wall time: 5.5 seconds
Features detected for doctesting:
Together with 30 optional doctests, the log-message shows the same result:
Using --optional=database_knotinfo,sage
Features to be detected: 4ti2,benzene,bliss,buckygen,conway_polynomials,csdp,database_cremona_ellcurve,database_cremona_mini_ellcurve,database_jones_numfield,database_knotinfo,dvipng,gfan,graphviz,imagemagick,jupymake,kenzo,latte_int,lrslib,mcqd,meataxe,msolve,nauty,palp,pandoc,pdf2svg,pdftocairo,phitigra,plantri,polytopes_db,polytopes_db_4d,pynormaliz,python_igraph,rubiks,sage.combinat,sage.geometry.polyhedron,sage.graphs,sage.groups,sage.plot,sage.rings.number_field,sage.rings.padics,sage.rings.real_double,sage.symbolic,sage_numerical_backends_coin,sagemath_doc_html,sphinx,tdlib
Doctesting 1 file.
sage -t --random-seed=32074844050499063835156990883996507444 src/sage/knots/knotinfo.py
[353 tests, 7.94 s]
----------------------------------------------------------------------
All tests passed!
----------------------------------------------------------------------
Total time for all tests: 8.1 seconds
cpu time: 7.9 seconds
cumulative wall time: 7.9 seconds
Features detected for doctesting:
This seems to be due to:
sage: from sage.doctest.external import AvailableSoftware
sage: S = AvailableSoftware()
sage: S._indices['database_knotinfo']
10
sage: S._seen[10]
0But this can be changed by performing a __contains__ test:
sage: 'database_knotinfo' in S
True
sage: S._seen[10]
1So it looks as if such a test is missing somewhere.
The line Features detected for doctesting: only reports the dynamically detected features, a subset of what is listed as Features to be detected:.
The statically known ones (from --optional) are not reported there.
This works as designed, but if you have a suggestion how it should be changed, we can change it.
Replying to @soehms:
If you have installed some optional feature
xyzon your system and forgot a# optional - xyzorxyz.is_present()somewhere in your change of code then this new doctest option will not bring this to your attention. That was what I hoped when I've first seen this ticket. Do you know, how this can be achieved without uninstalling the package?
The doctests marked # optional - xyz are disabled using --optional="!xyz" -- as your test above with --optional=!database_knotinfo,sage verified.
There is no mechanism to disable the actual feature in the Sage runtime. What xyz.is_present() does is not affected by the present ticket.
Reviewer: Sebastian Oehms
Replying to @mkoeppe:
The line Features detected for doctesting: only reports the dynamically detected features, a subset of what is listed as Features to be detected:. The statically known ones (from --optional) are not reported there.
I'm wondering what dynamically means here? From the code I displayed in my previous comment it appears that a feature is dynamically detected if it is tested to be contained in available_software. Where is that supposed to happen?
This works as designed, but if you have a suggestion how it should be changed, we can change it.
It's difficult to make a suggestion before I've completely understood the intention. According to the given phrase I assumed it would correspond to the Features to be detected. So, at least in order to resolve that irritation it would be enough to change the phrase into Dynamically detected features ... (whatever this means).
Replying to @soehms:
There is no mechanism to disable the actual feature in the Sage runtime. Whatxyz.is_present()does is not affected by the present ticket.
Perhaps I have in mind a new option sage -t --hide=FEATURES which would imply --optional='!FEATURE1,!FEATURE2,..'. Is there something against having such an additional option? I think that could be realized via a boolean _hidden in class Feature, which default is False and would be switched to True in case of this option.
Replying to @soehms:
Replying to @mkoeppe:
The line Features detected for doctesting: only reports the dynamically detected features, a subset of what is listed as Features to be detected:. The statically known ones (from --optional) are not reported there.
I'm wondering what dynamically means here? From the code I displayed in my previous comment it appears that a feature is dynamically detected if it is tested to be contained in
available_software. Where is that supposed to happen?
Sorry for using undefined terminology.
When you use sage -t --optional=sage,FEATURE, then FEATURE is "statically known".
When you use sage -t --optional=sage,optional and SPKG is the name of a normal package that is installed, then SPKG is "statically known".
Other features, by default those enumerated by sage.features.all.all_features except for those enumerated by sage.doctest.external.external_software, are subject to detection the first time that their name appears as an # optional - FEATURE annotation. (This is done by sage.doctest.parsing.SageDocTestParser.)
Replying to @mkoeppe:
Replying to @soehms:
Replying to @mkoeppe:
The line Features detected for doctesting: only reports the dynamically detected features, a subset of what is listed as Features to be detected:. The statically known ones (from --optional) are not reported there.
I'm wondering what dynamically means here? From the code I displayed in my previous comment it appears that a feature is dynamically detected if it is tested to be contained in
available_software. Where is that supposed to happen?Sorry for using undefined terminology.
When you use
sage -t --optional=sage,FEATURE, then FEATURE is "statically known".When you use
sage -t --optional=sage,optionaland SPKG is the name of a normal package that is installed, then SPKG is "statically known".Other features, by default those enumerated by
sage.features.all.all_featuresexcept for those enumerated bysage.doctest.external.external_software, are subject to detection the first time that their name appears as an# optional - FEATUREannotation. (This is done bysage.doctest.parsing.SageDocTestParser.)
Thanks for the explanation (I missed the fact that issuperset does the job)!
Replying to @soehms:
There is no mechanism to disable the actual feature in the Sage runtime. What
xyz.is_present()does is not affected by the present ticket.Perhaps I have in mind a new option
sage -t --hide=FEATURESwhich would imply--optional='!FEATURE1,!FEATURE2,..'. Is there something against having such an additional option?
Yes, I think something like this could make sense if there's a strong enough use case for it. It would probably be best to provide this mechanism also for non-doctest situations. So this would be a separate ticket.
Thanks for reviewing!
Replying to @mkoeppe:
Replying to @soehms:
There is no mechanism to disable the actual feature in the Sage runtime. What
xyz.is_present()does is not affected by the present ticket.Perhaps I have in mind a new option
sage -t --hide=FEATURESwhich would imply--optional='!FEATURE1,!FEATURE2,..'. Is there something against having such an additional option?Yes, I think something like this could make sense if there's a strong enough use case for it.
The use case I'm thinking of is the following: If you have installed an optional package xyz and run a test on your changes, say
sage -t src/sage/....py
then it might happen that you get All tests passed! here. But after
make xyz-clean
the same invocation of sage -t could find failures. Probably we will get more situations like that as modularization goes ahead. Surely, the patchbots will detect these. But it will be more convenient if you can detect these failures easily on your own system. I don't know how difficult it would be to fake the absence of an optional package in general. But in all cases where the Features functionality is used consequently, it should be doable.
It would probably be best to provide this mechanism also for non-doctest situations. So this would be a separate ticket.
Of course! After my vacation (that is in a couple of weeks) I will try to figure out how this could be done.
Changed branch from u/mkoeppe/sage__t___optional__sage__feature_ to ba86146
Thanks for this ticket. It will be quite useful for example to test that sage -t works even when let's say there is no latex on the machine (which is the case for the computer on which Volker makes tests before merging tickets).
Replying to @soehms:
Replying to @mkoeppe:
Replying to @soehms:
There is no mechanism to disable the actual feature in the Sage runtime. What
xyz.is_present()does is not affected by the present ticket.Perhaps I have in mind a new option
sage -t --hide=FEATURESwhich would imply--optional='!FEATURE1,!FEATURE2,..'. Is there something against having such an additional option?Yes, I think something like this could make sense if there's a strong enough use case for it.
The use case I'm thinking of is the following: If you have installed an optional package
xyzand run a test on your changes, saysage -t src/sage/....pythen it might happen that you get
All tests passed!here. But aftermake xyz-cleanthe same invocation of
sage -tcould find failures. Probably we will get more situations like that as modularization goes ahead. Surely, the patchbots will detect these. But it will be more convenient if you can detect these failures easily on your own system. I don't know how difficult it would be to fake the absence of an optional package in general. But in all cases where theFeaturesfunctionality is used consequently, it should be doable.It would probably be best to provide this mechanism also for non-doctest situations. So this would be a separate ticket.
Of course! After my vacation (that is in a couple of weeks) I will try to figure out how this could be done.
see #34185!