sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.42k stars 478 forks source link

Move Extension options from src/module_list.py to "distutils:" directives in the individual files (part 4: sage.rings) #29786

Closed mkoeppe closed 4 years ago

mkoeppe commented 4 years ago

Follow-up from #29706 (which is NOT a dependency of this ticket):

Taking care of sage.rings.*, except for those few that would need the additional cython_aliases from #29706.

CC: @kliem

Component: refactoring

Author: Matthias Koeppe

Branch/Commit: 1baaa68

Reviewer: Jonathan Kliem

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

mkoeppe commented 4 years ago

Branch: u/mkoeppe/move_extension_options_from_src_module_list_py_todistutils_directives_in_the_individual_filespart_4sagerings

mkoeppe commented 4 years ago

Author: Matthias Koeppe

mkoeppe commented 4 years ago

New commits:

aa75810src/module_list.py: Move options for most Extensions in sage.rings to distutils directives
mkoeppe commented 4 years ago

Commit: aa75810

mkoeppe commented 4 years ago

Description changed:

--- 
+++ 
@@ -1 +1,5 @@
-Follow-up from #29706
+Follow-up from #29706 (which is NOT a dependency of this ticket):
+
+Taking care of `sage.rings.*`, except for those few that would need the additional `cython_aliases` from #29706.
+
+
kliem commented 4 years ago
comment:3

LGTM.

As for testing, I rebuilt cython files in sage.rings on my machine and sage -t --long src/sage/rings/ passes.

kliem commented 4 years ago

Reviewer: Jonathan Kliem

mkoeppe commented 4 years ago
comment:4

Thank you!

mkoeppe commented 4 years ago
comment:5
File "src/sage/misc/sageinspect.py", line 28, in sage.misc.sageinspect
Failed example:
    sage_getsource(sage.rings.rational)[5:]
Expected:
    'Rational Numbers...'
Got:
    'tutils: libraries = ntl\nr"""\nRational Numbers\n\nAUTHORS:
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

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

43a9b16src/sage/misc/sageinspect.py: Fix up doctest that depends on a modified source file
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

Changed commit from aa75810 to 43a9b16

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

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

0b67deasrc/sage/libs/glpk/error.pyx: Make doctest more flexible
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

Changed commit from 43a9b16 to 0b67dea

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

Changed commit from 0b67dea to 43a9b16

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

mkoeppe commented 4 years ago
comment:9

Sorry, the last commit was supposed to go to a different ticket.

kliem commented 4 years ago
comment:10

The bots aren't happy yet:

sage -t --long src/sage/misc/sageinspect.py
**********************************************************************
File "src/sage/misc/sageinspect.py", line 28, in sage.misc.sageinspect
Failed example:
    sage_getsource(sage.rings.rational)[5:]
Expected:
    '# distutils: ... Rational Numbers...'
Got:
    'tutils: libraries = ntl\nr"""\nRational Numbers\n\nAUTHORS:\n\n- William Stein (2005): first version\n\n- William Stein (2006-02-22): floor and ceil (pure fast GMP versions).\n\n- Gonzalo Tornaria and William Stein (2006-03-02): greatly improved\n  python/GMP conversion; hashing\n\n- William Stein and Naqi Jaffery (2006-03-06): height, sqrt examples,\n  and improve behavior of sqrt.\n\n- David Harvey (2006-09-15): added nth_root\n\n- Pablo De Napoli (2007-04-01): corrected the implementations of\n  multiplicative_order, is_one; optimized __nonzero__ ; documented:\n  lcm,gcd\n\n- John Cremona (2009-05-15): added support for local and global\n  logarithmic heights.\n\n- Travis Scrimshaw (2012-10-18): Added doctests for full coverage.\n\n- Vincent Delecroix (2013): continued fraction\n\n- Vincent Delecroix (2017-05-03): faster integer-rational comparison\n\n- Vincent Klein (2017-05-11): add __mpq__() to class Rational\n\n- Vincent Klein (2017-05-22): Rational constructor support gmpy2.mpq\n  or gmpy2.mpz parameter. Add __mpz__ to class Rational.\n\nTESTS::\n\n    sage: a = -2/3\n    sage: a == loads(dumps(a))\n    True\n"""\n\n#*****************************************************************************\n#       Copyright (C) 2004, 2006 William Stein <wstein@gmail.com>\n#       Copyright (C) 2017 Vincent Delecroix <20100.delecroix@gmail.com>\n#\n#  Distributed under the terms of the GNU General Public License (GPL)\n#  as published by the Free Software Foundation; either version 2 of\n#  the License, or (at your option) any later version.\n#                  http://www.gnu.org/licenses/\n#*****************************************************************************\n\ncimport cython\nfrom cpython cimport *\nfrom cpython.object cimport Py_EQ, Py_NE\n\nfrom cysignals.signals cimport sig_on, sig_off\n\nimport sys\nimport operator\nimport fractions\n\nfrom sage.misc.mathml import mathml\nfrom sage.arith.long cimport pyobject_to_long, integer_check_long_py\nfrom sage.cpython.string cimport char_to_str, str_to_bytes\n\nimport sage.misc.misc as misc\nfrom sage.structure.sage_object cimport SageObject\nfrom sage.structure.richcmp cimport rich_to_bool_sgn\nimport sage.rings.rational_field\n\ncimport sage.rings.integer as integer\nfrom .integer cimport Integer\n\nfrom cypari2.paridecl cimport *\nfrom cypari2.gen cimport Gen as pari_gen\nfrom sage.libs.pari.convert_gmp cimport INT_to_mpz, INTFRAC_to_mpq, new_gen_from_mpq_t\n\nfrom .integer_ring import ZZ\nfrom sage.arith.rational_reconstruction cimport mpq_rational_reconstruction\n\nfrom sage.structure.coerce cimport is_numpy_type\n\nfrom sage.libs.gmp.pylong cimport mpz_set_pylong\n\nfrom sage.structure.coerce cimport coercion_model\nfrom sage.structure.element cimport Element\nfrom sage.structure.element import coerce_binop\nfrom sage.structure.parent cimport Parent\nfrom sage.categories.morphism cimport Morphism\nfrom sage.categories.map cimport Map\n\n\n\nimport sage.rings.real_mpfr\nimport sage.rings.real_double\nfrom libc.stdint cimport uint64_t\nfrom sage.libs.gmp.binop cimport mpq_add_z, mpq_mul_z, mpq_div_zz\n\nfrom cpython.int cimport PyInt_AS_LONG\n\ncimport sage.rings.fast_arith\nimport  sage.rings.fast_arith\n\ncdef sage.rings.fast_arith.arith_int ai\nai = sage.rings.fast_arith.arith_int()\n\ncdef object numpy_long_interface = {\'typestr\': \'=i4\' if sizeof(long) == 4 else \'=i8\' }\ncdef object numpy_int64_interface = {\'typestr\': \'=i8\'}\ncdef object numpy_object_interface = {\'typestr\': \'|O\'}\ncdef object numpy_double_interface = {\'typestr\': \'=f8\'}\n\nfrom libc.math cimport ldexp\nfrom sage.libs.gmp.all cimport *\n\ncimport gmpy2\ngmpy2.import_gmpy2()\n\n\ncdef class Rational(sage.structure.element.FieldElement)\n'
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

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

a5bc828src/sage/misc/sageinspect.py: Fixup fixup
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

Changed commit from 43a9b16 to a5bc828

mkoeppe commented 4 years ago
comment:12

Sorry... another round.

kliem commented 4 years ago
comment:13
src/sage/misc/sageinspect.py:121:1 'sys' imported but unused

I guess as you touched the file, the warning shows up.

kliem commented 4 years ago
comment:14

I tested it again and the bots are happy. I would propose removing this unneeded import and then you can put it back on positive review.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

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

1baaa68src/sage/misc/sageinspect.py: Remove unused import
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

Changed commit from a5bc828 to 1baaa68

mkoeppe commented 4 years ago
comment:17

Thank you!

vbraun commented 4 years ago

Changed branch from u/mkoeppe/move_extension_options_from_src_module_list_py_todistutils_directives_in_the_individual_filespart_4sagerings to 1baaa68