Rename test_... functions to avoid the naming scheme reserved by pytest

[mkoeppe]

sage -t throws errors on some files because pytest is configured to look for methods prefixed with test_ and treats them as pytest test functions/methods, which are then executed. However, sage's code base contains some functions that match this pattern without being pytests.

This leads to problems like

$ ./sage -tp src/sage/tests/cmdline.py
too many failed tests, not using stored timings
Running doctests with ID 2022-03-20-11-46-21-1dc8fb11.
Using --optional=4ti2,buckygen,ccache,cryptominisat,debugpy,e_antic,gap_packages,homebrew,igraph,jupymake,latte_int,libsemigroups,lidia,lrslib,meataxe,normaliz,pip,polytopes_db_4d,pynormaliz,sage,sage_spkg,texttable
Features to be detected: 4ti2,benzene,bliss,buckygen,conway_polynomials,csdp,database_cremona_ellcurve,database_cremona_mini_ellcurve,database_jones_numfield,database_knotinfo,dvipng,graphviz,imagemagick,jupymake,kenzo,latte_int,lrslib,mcqd,meataxe,nauty,palp,pandoc,pdf2svg,pdftocairo,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 using 8 threads.
sage -t --random-seed=23402257756506608859254049173870803147 src/sage/tests/cmdline.py
...
=================================================================================== test session starts ===================================================================================
platform darwin -- Python 3.10.2, pytest-7.0.1, pluggy-1.0.0
rootdir: /Users/mkoeppe/s/sage/sage-rebasing/worktree-gcc11/src, configfile: tox.ini
collected 1 item                                                                                                                                                                          

src/sage/tests/cmdline.py E                                                                                                                                                         [100%]

========================================================================================= ERRORS ==========================================================================================
____________________________________________________________________________ ERROR at setup of test_executable ____________________________________________________________________________
file /Users/mkoeppe/s/sage/sage-rebasing/worktree-gcc11/src/sage/tests/cmdline.py, line 61
  def test_executable(args, input="", timeout=100.0, pydebug_ignore_warnings=False, **kwds):
E       fixture 'args' not found
>       available fixtures: add_imports, cache, capfd, capfdbinary, caplog, capsys, capsysbinary, doctest_namespace, monkeypatch, pytestconfig, record_property, record_testsuite_property, record_xml_attribute, recwarn, tmp_path, tmp_path_factory, tmpdir, tmpdir_factory
>       use 'pytest --fixtures [testpath]' for help on them.

/Users/mkoeppe/s/sage/sage-rebasing/worktree-gcc11/src/sage/tests/cmdline.py:61
================================================================================= short test summary info =================================================================================
ERROR src/sage/tests/cmdline.py::test_executable
==================================================================================== 1 error in 0.01s =====================================================================================

We fix it in this ticket as follows:

• If the offending function is a proper (sage) test function, then change its name from test_xyz to _test_xyz (which also marks them as private)
• If the offending function is a "real" user-faced function, then explicitly exclude it from pytests test discovery.

See #31924 for further discussion on different approaches to fix the same issue including their pros and cons.

This is preparation for #33546.

Depends on #32975 Depends on #33612 Depends on #33617

CC: @tobiasdiez

Component: refactoring

Author: Tobias Diez

Branch/Commit: public/tests/pytest_wrong_test_methods @ 7644854

Reviewer: Sebastian Oehms, Matthias Koeppe

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

[slel]

comment:2

Good thing pycheck and pyright let us use check_... and right_...! : )

[tobiasdiez]

Description changed:

--- 
+++ 
@@ -1,3 +1,41 @@
-As suggested in [#31924 comment:37](https://github.com/sagemath/sage/issues/31924#comment:37)
+`sage -t` throws errors on some files because pytest is configured to look for methods prefixed with `test_` and treats them as pytest test functions/methods, which are then executed. However, sage's code base contains some functions that match this pattern without being pytests.
+
+This leads to problems like
+
+```
+$ ./sage -tp src/sage/tests/cmdline.py
+too many failed tests, not using stored timings
+Running doctests with ID 2022-03-20-11-46-21-1dc8fb11.
+Using --optional=4ti2,buckygen,ccache,cryptominisat,debugpy,e_antic,gap_packages,homebrew,igraph,jupymake,latte_int,libsemigroups,lidia,lrslib,meataxe,normaliz,pip,polytopes_db_4d,pynormaliz,sage,sage_spkg,texttable
+Features to be detected: 4ti2,benzene,bliss,buckygen,conway_polynomials,csdp,database_cremona_ellcurve,database_cremona_mini_ellcurve,database_jones_numfield,database_knotinfo,dvipng,graphviz,imagemagick,jupymake,kenzo,latte_int,lrslib,mcqd,meataxe,nauty,palp,pandoc,pdf2svg,pdftocairo,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 using 8 threads.
+sage -t --random-seed=23402257756506608859254049173870803147 src/sage/tests/cmdline.py
+...
+=================================================================================== test session starts ===================================================================================
+platform darwin -- Python 3.10.2, pytest-7.0.1, pluggy-1.0.0
+rootdir: /Users/mkoeppe/s/sage/sage-rebasing/worktree-gcc11/src, configfile: tox.ini
+collected 1 item                                                                                                                                                                          
+
+src/sage/tests/cmdline.py E                                                                                                                                                         [100%]
+
+========================================================================================= ERRORS ==========================================================================================
+____________________________________________________________________________ ERROR at setup of test_executable ____________________________________________________________________________
+file /Users/mkoeppe/s/sage/sage-rebasing/worktree-gcc11/src/sage/tests/cmdline.py, line 61
+  def test_executable(args, input="", timeout=100.0, pydebug_ignore_warnings=False, **kwds):
+E       fixture 'args' not found
+>       available fixtures: add_imports, cache, capfd, capfdbinary, caplog, capsys, capsysbinary, doctest_namespace, monkeypatch, pytestconfig, record_property, record_testsuite_property, record_xml_attribute, recwarn, tmp_path, tmp_path_factory, tmpdir, tmpdir_factory
+>       use 'pytest --fixtures [testpath]' for help on them.
+
+/Users/mkoeppe/s/sage/sage-rebasing/worktree-gcc11/src/sage/tests/cmdline.py:61
+================================================================================= short test summary info =================================================================================
+ERROR src/sage/tests/cmdline.py::test_executable
+==================================================================================== 1 error in 0.01s =====================================================================================
+```
+
+We fix it in this ticket as follows:
+- If the offending function is a proper (sage) test function, then change its name from `test_xyz` to `_test_xyz` (which also marks them as private)
+- If the offending function is a "real" user-faced function, then explicitly exclude it from pytests test discovery.
+
+See #31924 for further discussion on different approaches to fix the same issue including their pros and cons. 

 This is preparation for #33546.

[tobiasdiez]

Branch: public/tests/pytest_wrong_test_methods

[tobiasdiez]

Commit: 7060ea1

[tobiasdiez]

Author: Tobias Diez

[tobiasdiez]

Changed dependencies from #31924 to none

[tobiasdiez]

New commits:

59cabaa	Make test methods private
ba26086	Hide remaining methods starting with test from pytest
3dc0f63	Cleanup cmdline
321bd9d	Revert changes in test_class_pickling
db1d505	Fix tests in sageinspect
a09d5ec	Fix test_long
7060ea1	Fix imports in modular decomposition

[soehms]

comment:4

Yo should have a look at:

sage -t --warn-long 126.1 --random-seed=123007730607096856939441742800394041101 src/sage/tests/cmdline.py
**********************************************************************
File "src/sage/tests/cmdline.py", line 313, in sage.tests.cmdline._test_executable
Failed example:
    out.find("All tests passed!") >= 0
Expected:
    True
Got:
    False
**********************************************************************
File "src/sage/tests/cmdline.py", line 316, in sage.tests.cmdline._test_executable
Failed example:
    ret
Expected:
    0
Got:
    2
**********************************************************************
1 item had failures:
   2 of 217 in sage.tests.cmdline._test_executable
    [216 tests, 2 failures, 190.78 s]

and

sage -t --warn-long 125.1 --random-seed=216213492016970842904386788649176794439 src/sage/rings/tests.py
**********************************************************************
File "src/sage/rings/tests.py", line 416, in sage.rings.tests.?
Failed example:
    sage.rings.tests._test_karatsuba_multiplication(ZZ, 6, 5, verbose=True, seed=42)
Expected:
    _test_karatsuba_multiplication: ring=Univariate Polynomial Ring in x over Integer Ring, threshold=2
    (2*x^6 - x^5 - x^4 - 3*x^3 + 4*x^2 + 4*x + 1)*(4*x^4 + x^3 - 2*x^2 - 20*x + 3)
      (16*x^2)*(-41*x + 1)
      (x^6 + 2*x^5 + 8*x^4 - x^3 + x^2 + x)*(-x^2 - 4*x + 3)
      (-x^3 - x - 8)*(-1)
      (x - 1)*(-x^5 + 3*x^4 - x^3 + 2*x + 1)
      (x^3 + x^2 + x + 1)*(4*x^3 + 76*x^2 - x - 1)
      (x^6 - 5*x^4 - x^3 + 6*x^2 + 1)*(5*x^2 - x + 4)
      (3*x - 2)*(x - 1)
      (21)*(14*x^5 - x^2 + 4*x + 1)
      (12*x^5 - 12*x^2 + 2*x + 1)*(26*x^4 + x^3 + 1)
Got:
    test_karatsuba_multiplication: ring=Univariate Polynomial Ring in x over Integer Ring, threshold=2
      (2*x^6 - x^5 - x^4 - 3*x^3 + 4*x^2 + 4*x + 1)*(4*x^4 + x^3 - 2*x^2 - 20*x + 3)
      (16*x^2)*(-41*x + 1)
      (x^6 + 2*x^5 + 8*x^4 - x^3 + x^2 + x)*(-x^2 - 4*x + 3)
      (-x^3 - x - 8)*(-1)
      (x - 1)*(-x^5 + 3*x^4 - x^3 + 2*x + 1)
      (x^3 + x^2 + x + 1)*(4*x^3 + 76*x^2 - x - 1)
      (x^6 - 5*x^4 - x^3 + 6*x^2 + 1)*(5*x^2 - x + 4)
      (3*x - 2)*(x - 1)
      (21)*(14*x^5 - x^2 + 4*x + 1)
      (12*x^5 - 12*x^2 + 2*x + 1)*(26*x^4 + x^3 + 1)
**********************************************************************
1 item had failures:
   1 of   7 in sage.rings.tests.?
    [54 tests, 1 failure, 20.02 s]

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 7060ea1 to dec7b7b

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

dec7b7b

Fix doctest in rings

[tobiasdiez]

comment:6

@soehms, thanks for having a look. The first issue you mention also exists in develop and I made some progress in fixing it at #33550. The second issue is now fixed.

[mkoeppe]

comment:7

Testing with the command-line from the ticket description of #31924:

$ ./sage -tp src/sage/numerical/backends src/sage/symbolic/expression.pyx src/sage/manifolds/differentiable/symplectic_form_test.py 
too many failed tests, not using stored timings
Running doctests with ID 2022-03-24-12-10-39-2142352f.
Using --optional=4ti2,buckygen,ccache,cryptominisat,debugpy,e_antic,gap_packages,homebrew,igraph,jupymake,latte_int,libsemigroups,lidia,lrslib,meataxe,normaliz,pip,polytopes_db_4d,pynormaliz,sage,sage_spkg,texttable
Features to be detected: 4ti2,benzene,bliss,buckygen,conway_polynomials,csdp,database_cremona_ellcurve,database_cremona_mini_ellcurve,database_jones_numfield,database_knotinfo,dvipng,graphviz,imagemagick,jupymake,kenzo,latte_int,lrslib,mcqd,meataxe,nauty,palp,pandoc,pdf2svg,pdftocairo,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
Sorting sources by runtime so that slower doctests are run first....
Doctesting 27 files using 8 threads.
sage -t --random-seed=241138219171579094251943417982594901397 src/sage/numerical/backends/interactivelp_backend.pxd
    [0 tests, 0.00 s]
[...]
sage -t --random-seed=241138219171579094251943417982594901397 src/sage/numerical/backends/interactivelp_backend.pyx
    [266 tests, 5.13 s]
sage -t --random-seed=241138219171579094251943417982594901397 src/sage/symbolic/expression.pyx
    [3070 tests, 35.91 s]
----------------------------------------------------------------------
All tests passed!
----------------------------------------------------------------------
Total time for all tests: 36.7 seconds
    cpu time: 41.2 seconds
    cumulative wall time: 48.9 seconds
Features detected for doctesting: sage.rings.number_field
============================================================================================================ test session starts ============================================================================================================
platform darwin -- Python 3.10.2, pytest-7.0.1, pluggy-1.0.0
rootdir: /Users/mkoeppe/s/sage/sage-rebasing/worktree-gcc11/src, configfile: tox.ini
collected 0 items                                                                                                                                                                                                                           

=========================================================================================================== no tests ran in 0.01s ===========================================================================================================
ERROR: not found: /Users/mkoeppe/s/sage/sage-rebasing/worktree-gcc11/src/sage/symbolic/expression.pyx
(no name '/Users/mkoeppe/s/sage/sage-rebasing/worktree-gcc11/src/sage/symbolic/expression.pyx' in any of [])

[mkoeppe]

comment:8

There's a (trivial) merge conflict with #32975, best to merge in to make it easier to test.

[mkoeppe]

comment:9

Typo: skip_as_false_positve -> skip_as_false_positive (2 times)

[mkoeppe]

comment:10

Other than comment:7, comment:8, comment:9, it looks good.

I've tested it by itself (it makes a clear improvement - the remaining failures discussed in comment:6 and comment:7 are also present in the current beta).

It also works well with #31924 merged (it does not create new problems).

[mkoeppe]

Reviewer: Sebastian Oehms, Matthias Koeppe

[soehms]

comment:13

Replying to @tobiasdiez:

@soehms, thanks for having a look. The first issue you mention also exists in develop and I made some progress in fixing it at #33550. The second issue is now fixed.

Merging with #33550 I had to resolve some conflics. So I think it would make sence to have it here as dependency.

Im not really familiar with pytest. Thus, I can check that this branch does what it is supposed to. But I'm not the right person to evaluate the code. I think it would be helpful, if the hook function pytest_collection_modifyitems (and maybe other function and classes in conftest.py, as well) would have a docstring explaining what this hook is good for (with pointers to the documentation) and why we need to use it here (in more than just one line).

Together with #33550 the behavior with respect to src/sage/tests/cmdline.py remains unchanged. Further, I get the following warnings:

Doctesting 3 files using 4 threads.
sage -t --long --warn-long 124.6 --random-seed=329268236894550985110801561272292236637 src/sage/misc/explain_pickle.py
    [329 tests, 0.52 s]
sage -t --long --warn-long 124.6 --random-seed=329268236894550985110801561272292236637 src/sage/schemes/elliptic_curves/kraus.py
    [142 tests, 24.97 s]
sage -t --long --warn-long 124.6 --random-seed=329268236894550985110801561272292236637 src/sage/symbolic/relation.py
    [393 tests, 28.02 s]
----------------------------------------------------------------------
All tests passed!
----------------------------------------------------------------------
Total time for all tests: 28.6 seconds
    cpu time: 51.6 seconds
    cumulative wall time: 53.5 seconds
Features detected for doctesting:
==================================================================================================================== test session starts ====================================================================================================================
platform linux -- Python 3.10.2, pytest-7.1.1, pluggy-1.0.0
rootdir: /home/sebastian/devel/sage/src, configfile: tox.ini
collected 7 items

src/sage/misc/explain_pickle.py s                                                                                                                                                                                                                     [ 14%]
src/sage/schemes/elliptic_curves/kraus.py sssss                                                                                                                                                                                                       [ 85%]
src/sage/symbolic/relation.py s                                                                                                                                                                                                                       [100%]

===================================================================================================================== warnings summary ======================================================================================================================
src/sage/misc/explain_pickle.py:2723
  /home/sebastian/devel/sage/src/sage/misc/explain_pickle.py:2723: PytestCollectionWarning: cannot collect test class 'TestReduceGetinitargs' because it has a __init__ constructor (from: sage/misc/explain_pickle.py)
    class TestReduceGetinitargs:

src/sage/misc/explain_pickle.py:2775
  /home/sebastian/devel/sage/src/sage/misc/explain_pickle.py:2775: PytestCollectionWarning: cannot collect test class 'TestReduceNoGetinitargs' because it has a __init__ constructor (from: sage/misc/explain_pickle.py)
    class TestReduceNoGetinitargs:

src/sage/misc/explain_pickle.py:2813
  /home/sebastian/devel/sage/src/sage/misc/explain_pickle.py:2813: PytestCollectionWarning: cannot collect test class 'TestAppendList' because it has a __init__ constructor (from: sage/misc/explain_pickle.py)
    class TestAppendList(list):

src/sage/misc/explain_pickle.py:2861
  /home/sebastian/devel/sage/src/sage/misc/explain_pickle.py:2861: PytestCollectionWarning: cannot collect test class 'TestAppendNonlist' because it has a __init__ constructor (from: sage/misc/explain_pickle.py)
    class TestAppendNonlist(object):

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
============================================================================================================== 7 skipped, 4 warnings in 0.18s ===============================================================================================================

Are these expected?

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

5851f01	Improve doctest interaction with pytest
e717715	Merge remote-tracking branch 'origin/develop' into public/tests/doctests_pytest
9c5d5a9	Fix docs
0c7bf17	Add documentation for test:pytest
01f376c	Use endswith to test for pytest files
1b7da1b	Merge remote-tracking branch 'trac/public/tests/doctests_pytest' into public/tests/pytest_wrong_test_methods
3cab51d	Fix spelling
e6312ec	Add docs to newly added pytest hook

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from dec7b7b to e6312ec

[tobiasdiez]

comment:15

Replying to @mkoeppe:

There's a (trivial) merge conflict with #32975, best to merge in to make it easier to test.

Merged!

[tobiasdiez]

Dependencies: #32975

[tobiasdiez]

comment:17

Replying to @mkoeppe:

Typo: skip_as_false_positve -> skip_as_false_positive (2 times)

Fixed!

[tobiasdiez]

comment:18

Replying to @mkoeppe:

Testing with the command-line from the ticket description of #31924:

$ ./sage -tp src/sage/numerical/backends src/sage/symbolic/expression.pyx src/sage/manifolds/differentiable/symplectic_form_test.py 

This is now #33560.

[tobiasdiez]

comment:19

Replying to @soehms:

Replying to @tobiasdiez:

@soehms, thanks for having a look. The first issue you mention also exists in develop and I made some progress in fixing it at #33550. The second issue is now fixed.

Merging with #33550 I had to resolve some conflics. So I think it would make sence to have it here as dependency.

Since both tickets are still under review, I would propose to merge the first positively-reviewed ticket in the other one (to facilitate reviewing). Is this okay?

[tobiasdiez]

comment:20

Replying to @soehms:

I think it would be helpful, if the hook function pytest_collection_modifyitems (and maybe other function and classes in conftest.py, as well) would have a docstring explaining what this hook is good for (with pointers to the documentation) and why we need to use it here (in more than just one line).

Good suggestion! Implemented.

[tobiasdiez]

comment:21

Replying to @soehms:

Are these expected?

Not really. It's the same issue as here with the test functions. These classes are discovered by pytest, which treats them as pytest test classes and fails. I've opened #33561 for this.

[mkoeppe]

comment:22

LGTM. You can set to positive review or wait if Sebastian has further comments.

[soehms]

comment:23

Replying to @tobiasdiez:

Replying to @soehms: Since both tickets are still under review, I would propose to merge the first positively-reviewed ticket in the other one (to facilitate reviewing). Is this okay?

Yes, thats o.K.

[soehms]

comment:24

Replying to @mkoeppe:

LGTM. You can set to positive review or wait if Sebastian has further comments.

LGTM as well!

[tobiasdiez]

comment:25

Thanks for the quick review!

[soehms]

comment:26

Sorry, that was a little bit too fast. You accidently dropped a line from the doctest in src/sage/tests/cmdline.py. The first failure (line 313) I reported in comment:4 has not been in develop.

@@ -308,12 +308,11 @@ def test_executable(args, input="", timeout=100.0, pydebug_ignore_warnings=False
     Now test my_script.sage and the preparsed version my_script.sage.py::

-        sage: (out, err, ret) = test_executable(["sage", "-t", script])
         sage: ret
         0
         sage: out.find("All tests passed!") >= 0
         True
-        sage: (out, err, ret) = test_executable(["sage", "-t", script_py])
+        sage: (out, err, ret) = _test_executable(["sage", "-t", script_py])
         sage: ret

As soon as this is fixed you may switch back to positive review.

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

6768bd2

src/sage/tests/cmdline.py: Restore dropped line in doctest

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from e6312ec to 6768bd2

[tobiasdiez]

comment:29

Good that you spotted this accident, and thanks for already fixing it!

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

7644854

Fix typo

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 6768bd2 to 7644854

[tobiasdiez]

comment:31

There was still a small typo in the function name, which I've fixed now.

[GMS103]

comment:32

FWIW, positive review for SageMath 9.6.beta6 plus #31924 and #33549 plus make pytest on Apple Silicon M1 Macs with Homebrew up to date, both macOS 11.6.5 (Big Sur) with Xcode 13.2.1 and macOS 12.3 (Monterey) with Xcode 13.3.

[tscrim]

comment:33

I am not sure I support giving pytest such high priority over namespace allocation. This is a fairly major change in policy that should be discussed on sage-devel first. Furthermore, you are also hiding functions that used to be public too, which violates Sage's deprecation policy

[tobiasdiez]

comment:34

In #31924 comment:43, Matthias argues that

I think it's fine to rename them if they are only used by doctests in the same module. Then we might argue that it was not really intended to be in the public interface and should have been named _test....

I agree with him that these test-related methods should have been private from the get-go. Especially since most of them contain an explicit disclaimer ala "this is intended only for usage in doctests".

But if you really have issues with the rename and think it's the right procedure, I can start a discussion on the mailing list.

[vbraun]

Changed branch from public/tests/pytest_wrong_test_methods to 7644854

[vbraun]

Changed branch from 7644854 to public/tests/pytest_wrong_test_methods

[vbraun]

comment:37

Well tests work... let me know when it is ready to be merged.

[tscrim]

comment:39

Replying to @tobiasdiez:

In #31924 comment:43, Matthias argues that

I think it's fine to rename them if they are only used by doctests in the same module. Then we might argue that it was not really intended to be in the public interface and should have been named _test....

I agree with him that these test-related methods should have been private from the get-go. Especially since most of them contain an explicit disclaimer ala "this is intended only for usage in doctests".

It does not matter what they should have been, just what they are.

But if you really have issues with the rename and think it's the right procedure, I can start a discussion on the mailing list.

Yes: comment:33

[mkoeppe]

comment:40

Instead of a policy discussion, perhaps can we move the ticket forward by setting up deprecated aliases for the renamed functions.

[tscrim]

comment:41

I strongly think we should not give any package, much less an optional package, any priority over names of public functions without a very good reason. I am also not convinced that we want to move forward with pytest becoming standard either. Both of these need to be discussed on sage-devel, with the first issue for this ticket. You cannot sidestep this. Deprecating the functions is efffectively the same as renaming them as it marks a major policy change.

[tscrim]

comment:42

Well, function names that are generic. If they are sufficiently unique to the package, then that is fine with me.

[mkoeppe]

comment:43

All of them are unique to the module that they are defined in, and their purpose is to support doctests in the same module. Renaming them from test_... to _test_... is justified by existing policies, I'd say.

[tscrim]

comment:44

Yes, but that is a non sequitur. I agree that renaming them (with deprecation) is within the current policy, but it is the reason for these changes that is the issue. It is about the conflict with an optional package's conventions that are fully reserve a certain class of (natural) function names. Enforcing this (which is done here across the entire Sage library) would give pytest very special privileges and status within Sage.

[mkoeppe]

comment:45

I have split out #33612 for the strategy proposed in comment:40, comment:43. As you will see in the ticket description, it is an improvement to the Sage library independent from its motivation for pytest.