Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
mkoeppe opened this issue Jun 3, 2020 · 26 comments

Comments

@mkoeppe
Copy link
Member

mkoeppe commented Jun 3, 2020

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 mkoeppe added this to the sage-9.2 milestone Jun 3, 2020
@mkoeppe
Copy link
Member Author

mkoeppe commented Jun 3, 2020

Author: Matthias Koeppe

@mkoeppe
Copy link
Member Author

mkoeppe commented Jun 3, 2020

New commits:

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

@mkoeppe
Copy link
Member Author

mkoeppe commented Jun 3, 2020

Commit: aa75810

@mkoeppe

This comment has been minimized.

@kliem
Copy link
Contributor

kliem commented Jun 4, 2020

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
Copy link
Contributor

kliem commented Jun 4, 2020

Reviewer: Jonathan Kliem

@mkoeppe
Copy link
Member Author

mkoeppe commented Jun 4, 2020

comment:4

Thank you!

@mkoeppe
Copy link
Member Author

mkoeppe commented Jun 9, 2020

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:

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 9, 2020

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 9, 2020

Changed commit from aa75810 to 43a9b16

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 9, 2020

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

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 9, 2020

Changed commit from 43a9b16 to 0b67dea

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 9, 2020

Changed commit from 0b67dea to 43a9b16

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 9, 2020

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

@mkoeppe
Copy link
Member Author

mkoeppe commented Jun 9, 2020

comment:9

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

@kliem
Copy link
Contributor

kliem commented Jun 13, 2020

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 <[email protected]>\n#       Copyright (C) 2017 Vincent Delecroix <[email protected]>\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'

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 13, 2020

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

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 13, 2020

Changed commit from 43a9b16 to a5bc828

@mkoeppe
Copy link
Member Author

mkoeppe commented Jun 13, 2020

comment:12

Sorry... another round.

@kliem
Copy link
Contributor

kliem commented Jun 13, 2020

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
Copy link
Contributor

kliem commented Jun 13, 2020

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.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 13, 2020

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

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 13, 2020

Changed commit from a5bc828 to 1baaa68

@mkoeppe
Copy link
Member Author

mkoeppe commented Jun 13, 2020

comment:17

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants