Follow up #36741: improve code structure

[soehms]

From #36741 (comment)

Overall, I would suggest to remove the direct and unconditional use of multiprocessing from sage.features.
Perhaps sage.doctest can put such Value attributes into features that are to be hidden.

This PR implements the suggestion made in #36741 (comment)

I looked a little bit into it and here's an idea:

• In features, implement a simple hide() / unhide() / is_hidden() interface.
• The only state is _hidden, not shared (for parallel doctesting this will be set before fork so it's ok)
• Implement AvailableSoftware.hidden() similar to AvailableSoftware.seen() so the logic stays in that class and internals don't leak to sage.doctest.control (if necessary use _seen[idx] to record hidden state and/or number of hidings, or add another shared array)

Fixes #37905

📝 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 accordingly.

⌛ Dependencies

[mkoeppe]

LGTM, thanks

[soehms]

LGTM, thanks

Thanks, too!

[vbraun]

I'm getting this in parallel tests, but passes in isolation:

sage -t --long --warn-long 43.9 --random-seed=123 src/sage/doctest/external.py
**********************************************************************
File "src/sage/doctest/external.py", line 518, in sage.doctest.external.AvailableSoftware.hidden
Failed example:
    available_software.hidden()
Expected:
    [...'conway_polynomials',
     'database_cremona_mini_ellcurve',
     'database_ellcurves',
     'database_graphs'...]
Got:
    ['database_cremona_mini_ellcurve', 'database_ellcurves', 'database_graphs']
**********************************************************************

[github-actions]

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

[tornaria]

I'm getting this in parallel tests, but passes in isolation:

sage -t --long --warn-long 43.9 --random-seed=123 src/sage/doctest/external.py
**********************************************************************
File "src/sage/doctest/external.py", line 518, in sage.doctest.external.AvailableSoftware.hidden
Failed example:
    available_software.hidden()
Expected:
    [...'conway_polynomials',
     'database_cremona_mini_ellcurve',
     'database_ellcurves',
     'database_graphs'...]
Got:
    ['database_cremona_mini_ellcurve', 'database_ellcurves', 'database_graphs']
**********************************************************************

I can't reproduce this. Could you give us more details and/or share the head of the tree where you see this?

[vbraun]

I got this twice in a row when running make distclean && make && make ptestlong

But in general can we make the test maybe less fragile, is it really important that those 4 items have to be consecutively in there? Maybe you just want to check that one is in there so hidden works at all?

[soehms]

is it really important that those 4 items have to be consecutively in there? Maybe you just want to check that one is in there so hidden works at all?

The fact that conway_polynomials is missing from the answer is indeed strange. If this would happen when a developer runs sage -tp --hide=conway_polynomials we would give him a wrong feedback.

[soehms]

The current commit addresses #37905 in the context of this PR. As mentioned in #37857 (comment) the connection betwee the content of the variable _seen and the result of the method __contains__ turns out to be problematic when using the hide option since this option may cause an available feature to be temporarily unavailable. Therefore, I suggest decoupling the result from the content of _seen.

Note that there is no real need to cache the result of self._features[idx].is_present() as it is already cached in the _is_present method of the Feature class. From this point of view it seems that there is no real need to set _seen to -1. @kwankyu, since you introduced this variable in March 2016 (in this commit), can you please confirm that I'm right?

Likewise, there is no need to set _hidden to -1. I couldn't reproduce the failure that @vbraun got (see #37737 (comment)), but I suspect this happened because _hidden for the feature conway_polynomials was set to -1 (by a thread calling __contains__ as the feature was not hidden). Therefore, there is a good chance that this failure will no longer occur.

[mkoeppe]

test-mod shows this failure. This is a separate test of the modularized distribution sagemath-categories.

[sagemath_categories-10.4.beta5] [spkg-check] sage -t --random-seed=207122778768807178970207259884772121442 --environment=sage.all__sagemath_categories sage/doctest/external.py
[sagemath_categories-10.4.beta5] [spkg-check] **********************************************************************
[sagemath_categories-10.4.beta5] [spkg-check] File "sage/doctest/external.py", line 519, in sage.doctest.external.AvailableSoftware.hidden
[sagemath_categories-10.4.beta5] [spkg-check] Failed example:
[sagemath_categories-10.4.beta5] [spkg-check]     sorted(available_software.hidden())
[sagemath_categories-10.4.beta5] [spkg-check] Expected:
[sagemath_categories-10.4.beta5] [spkg-check]     [...'conway_polynomials',...
[sagemath_categories-10.4.beta5] [spkg-check]      'database_cremona_mini_ellcurve',...
[sagemath_categories-10.4.beta5] [spkg-check]      'database_ellcurves',...
[sagemath_categories-10.4.beta5] [spkg-check]      'database_graphs'...]
[sagemath_categories-10.4.beta5] [spkg-check] Got:
[sagemath_categories-10.4.beta5] [spkg-check]     []
[sagemath_categories-10.4.beta5] [spkg-check] **********************************************************************

[soehms]

Thanks!