#29786 closed enhancement (fixed)

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

Reported by: mkoeppe Owned by:
Priority: major Milestone: sage-9.2
Component: refactoring Keywords:
Cc: gh-kliem Merged in:
Authors: Matthias Koeppe Reviewers: Jonathan Kliem
Report Upstream: N/A Work issues:
Branch: 1baaa68 (Commits, GitHub, GitLab) Commit: 1baaa68a0fde79d25081c463fed8657c44a43762
Dependencies: Stopgaps:

Status badges

Description (last modified by mkoeppe)

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.

Change History (18)

comment:1 Changed 17 months ago by mkoeppe

  • Branch set to u/mkoeppe/move_extension_options_from_src_module_list_py_to__distutils___directives_in_the_individual_files__part_4__sage_rings_

comment:2 Changed 17 months ago by mkoeppe

  • Authors set to Matthias Koeppe
  • Commit set to aa7581059a44bf380614967bfb279f2c88969b74
  • Description modified (diff)
  • Status changed from new to needs_review

New commits:

aa75810src/module_list.py: Move options for most Extensions in sage.rings to distutils directives

comment:3 Changed 17 months ago by gh-kliem

  • Reviewers set to Jonathan Kliem
  • Status changed from needs_review to positive_review

LGTM.

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

comment:4 Changed 17 months ago by mkoeppe

Thank you!

comment:5 Changed 17 months ago by mkoeppe

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:

comment:6 Changed 17 months ago by git

  • Commit changed from aa7581059a44bf380614967bfb279f2c88969b74 to 43a9b169f4030b9a036f3c0e3f94214dbc4f6ab7
  • Status changed from positive_review to needs_review

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

comment:7 Changed 17 months ago by git

  • Commit changed from 43a9b169f4030b9a036f3c0e3f94214dbc4f6ab7 to 0b67deab8412c2c34a0a17ae50cda8f2042a2c30

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

0b67deasrc/sage/libs/glpk/error.pyx: Make doctest more flexible

comment:8 Changed 17 months ago by git

  • Commit changed from 0b67deab8412c2c34a0a17ae50cda8f2042a2c30 to 43a9b169f4030b9a036f3c0e3f94214dbc4f6ab7

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

comment:9 Changed 17 months ago by mkoeppe

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

comment:10 Changed 17 months ago by gh-kliem

  • Status changed from needs_review to needs_work

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'

comment:11 Changed 17 months ago by git

  • Commit changed from 43a9b169f4030b9a036f3c0e3f94214dbc4f6ab7 to a5bc828338393e248e1d9e9e9719f9ba22aa7b4e

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

a5bc828src/sage/misc/sageinspect.py: Fixup fixup

comment:12 Changed 17 months ago by mkoeppe

  • Status changed from needs_work to needs_review

Sorry... another round.

comment:13 Changed 17 months ago by gh-kliem

src/sage/misc/sageinspect.py:121:1 'sys' imported but unused

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

comment:14 Changed 17 months ago by gh-kliem

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.

comment:15 Changed 17 months ago by git

  • Commit changed from a5bc828338393e248e1d9e9e9719f9ba22aa7b4e to 1baaa68a0fde79d25081c463fed8657c44a43762

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

1baaa68src/sage/misc/sageinspect.py: Remove unused import

comment:16 Changed 17 months ago by gh-kliem

  • Status changed from needs_review to positive_review

comment:17 Changed 17 months ago by mkoeppe

Thank you!

comment:18 Changed 16 months ago by vbraun

  • Branch changed from u/mkoeppe/move_extension_options_from_src_module_list_py_to__distutils___directives_in_the_individual_files__part_4__sage_rings_ to 1baaa68a0fde79d25081c463fed8657c44a43762
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.