sagemath / sage

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

is_integer method is missing on integers #15500

Closed ppurka closed 10 years ago

ppurka commented 10 years ago

From google spreadsheet which no one reads X-(

Instead integers have is_integral.

sage: sqrt(3).is_integer()
False
sage: sqrt(4).is_integer()
---------------------------------------------------------------------------
AttributeError Traceback (most recent call last)
<ipython-input-2-f8523f2586e7> in <module>()
----> 1 sqrt(Integer(4)).is_integer()

/opt/sage-5.11/local/lib/python2.7/site-packages/sage/structure/element.so in sage.structure.element.Element.__getattr__ (sage/structure/element.c:3871)()

/opt/sage-5.11/local/lib/python2.7/site-packages/sage/structure/misc.so in sage.structure.misc.getattr_from_other_class (sage/structure/misc.c:1696)()

AttributeError: 'sage.rings.integer.Integer' object has no attribute 'is_integer'

Component: algebra

Author: Rajesh Veeranki

Branch/Commit: 4d00b30

Reviewer: Punarbasu Purkayastha, Jeroen Demeyer

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

3shv commented 10 years ago

Branch: u/Rajesh_Veeranki/ticket/15500

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

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

4c4826eAdded is_integral() function to integer class
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

Commit: 4c4826e

ppurka commented 10 years ago
comment:4

This needs more work actually, so that other fields also give proper results. For example:

sage: a = RR(3)
sage: a = AA(3)
sage: a = RDF(3)

and so on.. The correct fix in those fields would be either to have a function alias if is_integral is already defined in the field (for example QQ)

is_integer = is_integral

or have a code like this in that field

def is_integer(self):
    return self in ZZ
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

Changed commit from 4c4826e to 6ca987d

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

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

6ca987dAdded is_integer() method to integer class
ppurka commented 10 years ago
comment:6

Hi Rajesh, seems like you are a new developer to Sage. I think this is a good ticket for you since it will need you to go through many different field implementations and introduce this small method into them.

You do not need to make all the changes in one go. You can add a new commit, introducing the method in a different field. At the end when you do sage --dev push it will push all your commits on to the ticket.

When you are done making the changes, you should set the status of the ticket to "needs review". This can be done from the command line using the following command when your branch is ticket/15500 (or it corresponds to this ticket):

$ sage --dev needs-review

Alternatively, you can log in to trac via the browser and set it yourself. If you log in to trac via the browser you can also set your author name in the author field of the ticket. This should match your author name in git, which I noticed you have set up properly.

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

Changed commit from 6ca987d to db07cb1

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

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

36fab41Added alias function is_integer() in rational.pyx
4a87faeAdded is_integer() function to QQbar.py
23a67beAdded is_integer() function to check if its an integer,in real_double.pyx
9ed3084Added is_integer() function to complex_double.pyx
db07cb1Added is_integer() function to complex_number.pyx
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

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

85f8025Added is_integer() function to Real Number class
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

Changed commit from db07cb1 to 85f8025

3shv commented 10 years ago
comment:9

Hi ppurka,

I'm a final year CSE student from IITB and new to the dev process.Thanks for guiding me. I've incoporated changes to the fields adding the is_integer() function.I think i covered all the fields,if anything is missing please let me know.Thanks!!

P.S: Yesterday, i have deleted the branch using the command < git push trac :branchName > which messed up the ticket,so i had to repush again! Sorry for the trouble,if i've caused any! I'm not using ./sage -dev as it uses git protocol, and my university only allows http,https.

ppurka commented 10 years ago
comment:11

Thanks for the patches!

There are two doctest failures. It needs a bit more work. We will also have to check that it does not affect anything else adversely:

sage -t --long src/sage/structure/element.pyx
**********************************************************************
File "src/sage/structure/element.pyx", line 354, in sage.structure.element.Element.__dir__
Failed example:
    dir(1/2)
Expected:
    ['N', ..., 'is_idempotent', 'is_integral', ...]
Got:
    ['N', '__abs__', '__add__', '__array_interface__', '__class__', '__cmp__', '__copy__', '__delattr__', '__dict__', '__dir__', '__div__', '__doc__', '__eq__', '__float__', '__floordiv__', '__foo', '__format__', '__ge__', '__getattribute__', '__getitem__', '__getstate__', '__gt__', '__hash__', '__iadd__', '__idiv__', '__imul__', '__index__', '__init__', '__int__', '__invert__', '__isub__', '__le__', '__long__', '__lshift__', '__lt__', '__mod__', '__module__', '__mul__', '__ne__', '__neg__', '__new__', '__nonzero__', '__pos__', '__pow__', '__pyx_vtable__', '__radd__', '__rdiv__', '__reduce__', '__reduce_ex__', '__repr__', '__rfloordiv__', '__rlshift__', '__rmod__', '__rmul__', '__rpow__', '__rrshift__', '__rshift__', '__rsub__', '__rtruediv__', '__rxor__', '__setattr__', '__setstate__', '__sizeof__', '__str__', '__sub__', '__subclasshook__', '__truediv__', '__weakref__', '__xor__', '_act_on_', '_acted_upon_', '_add_', '_add_parent', '_ascii_art_', '_axiom_', '_axiom_init_', '_bnfisnorm', '_cmp_', '_coeff_repr', '_derivative', '_div_', '_dummy_attribute', '_fricas_', '_fricas_init_', '_gap_', '_gap_init_', '_giac_', '_giac_init_', '_gp_', '_gp_init_', '_iadd_', '_idiv_', '_ilmul_', '_im_gens_', '_imul_', '_integer_', '_interface_', '_interface_init_', '_interface_is_cached_', '_is_atomic', '_isub_', '_kash_', '_kash_init_', '_latex_', '_latex_coeff_repr', '_lcm', '_lmul_', '_macaulay2_', '_macaulay2_init_', '_magma_init_', '_make_new_with_parent_c', '_maple_', '_maple_init_', '_mathematica_', '_mathematica_init_', '_mathml_', '_maxima_', '_maxima_init_', '_maxima_lib_', '_maxima_lib_init_', '_mpmath_', '_mul_', '_mul_parent', '_neg_', '_octave_', '_octave_init_', '_pari_', '_pari_init_', '_pow_', '_pow_naive', '_r_init_', '_reduce_set', '_reduction', '_repr_', '_richcmp_', '_rmul_', '_sage_', '_sage_input_', '_sage_src_lines_', '_set_parent', '_singular_', '_singular_init_', '_sub_', '_sympy_', '_test_category', '_test_eq', '_test_nonzero_equal', '_test_not_implemented_methods', '_test_pickling', '_tester', 'abs', 'absolute_norm', 'additive_order', 'base_extend', 'base_ring', 'cartesian_product', 'category', 'ceil', 'charpoly', 'conjugate', 'content', 'db', 'denom', 'denominator', 'derivative', 'divides', 'dump', 'dumps', 'factor', 'floor', 'gamma', 'gcd', 'global_height', 'global_height_arch', 'global_height_non_arch', 'height', 'imag', 'inverse_mod', 'is_S_integral', 'is_S_unit', 'is_idempotent', 'is_integer', 'is_integral', 'is_nilpotent', 'is_norm', 'is_nth_power', 'is_one', 'is_padic_square', 'is_perfect_power', 'is_square', 'is_unit', 'is_zero', 'lcm', 'list', 'local_height', 'local_height_arch', 'minpoly', 'mod', 'mod_ui', 'multiplicative_order', 'n', 'norm', 'nth_root', 'numer', 'numerator', 'numerical_approx', 'ord', 'order', 'parent', 'partial_fraction_decomposition', 'period', 'prime_to_S_part', 'quo_rem', 'real', 'relative_norm', 'rename', 'reset_name', 'round', 'save', 'sign', 'sqrt', 'sqrt_approx', 'squarefree_part', 'str', 'subs', 'substitute', 'support', 'trace', 'val_unit', 'valuation', 'version', 'xgcd']
**********************************************************************
File "src/sage/structure/element.pyx", line 359, in sage.structure.element.Element.__dir__
Failed example:
    1.__dir__()
Expected:
    ['N', ..., 'is_idempotent', 'is_integral', ...]
Got:
    ['N', '__abs__', '__add__', '__and__', '__array_interface__', '__class__', '__cmp__', '__copy__', '__delattr__', '__dict__', '__dir__', '__div__', '__divmod__', '__doc__', '__eq__', '__float__', '__floordiv__', '__foo', '__format__', '__ge__', '__getattribute__', '__getstate__', '__gt__', '__hash__', '__hex__', '__iadd__', '__idiv__', '__imul__', '__index__', '__init__', '__int__', '__invert__', '__isub__', '__le__', '__long__', '__lshift__', '__lt__', '__mod__', '__module__', '__mul__', '__ne__', '__neg__', '__new__', '__nonzero__', '__oct__', '__or__', '__pos__', '__pow__', '__pyx_vtable__', '__radd__', '__rand__', '__rdiv__', '__rdivmod__', '__reduce__', '__reduce_ex__', '__repr__', '__rfloordiv__', '__rlshift__', '__rmod__', '__rmul__', '__ror__', '__rpow__', '__rrshift__', '__rshift__', '__rsub__', '__rtruediv__', '__rxor__', '__setattr__', '__setstate__', '__sizeof__', '__str__', '__sub__', '__subclasshook__', '__truediv__', '__weakref__', '__xor__', '_act_on_', '_acted_upon_', '_add_', '_add_parent', '_ascii_art_', '_axiom_', '_axiom_init_', '_bnfisnorm', '_cmp_', '_coeff_repr', '_div_', '_dummy_attribute', '_exact_log_log2_iter', '_exact_log_mpfi_log', '_fricas_', '_fricas_init_', '_gap_', '_gap_init_', '_giac_', '_giac_init_', '_gp_', '_gp_init_', '_iadd_', '_idiv_', '_ilmul_', '_im_gens_', '_imul_', '_interface_', '_interface_init_', '_interface_is_cached_', '_is_atomic', '_isub_', '_kash_', '_kash_init_', '_latex_', '_latex_coeff_repr', '_lcm', '_lmul_', '_macaulay2_', '_macaulay2_init_', '_magma_init_', '_make_new_with_parent_c', '_maple_', '_maple_init_', '_mathematica_', '_mathematica_init_', '_mathml_', '_maxima_', '_maxima_init_', '_maxima_lib_', '_maxima_lib_init_', '_mpmath_', '_mul_', '_mul_parent', '_neg_', '_octave_', '_octave_init_', '_pari_', '_pari_init_', '_pow_', '_pow_naive', '_r_init_', '_reduction', '_repr_', '_richcmp_', '_rmul_', '_rpy_', '_sage_', '_sage_input_', '_sage_src_lines_', '_set_parent', '_shift_helper', '_singular_', '_singular_init_', '_sub_', '_sympy_', '_test_category', '_test_eq', '_test_nonzero_equal', '_test_not_implemented_methods', '_test_pickling', '_tester', '_valuation', '_xgcd', 'abs', 'additive_order', 'base_extend', 'base_ring', 'binary', 'binomial', 'bits', 'cartesian_product', 'category', 'ceil', 'conjugate', 'coprime_integers', 'crt', 'db', 'degree', 'denominator', 'digits', 'divide_knowing_divisible_by', 'divides', 'divisors', 'dump', 'dumps', 'exact_log', 'exp', 'factor', 'factorial', 'floor', 'gamma', 'gcd', 'global_height', 'imag', 'inverse_mod', 'inverse_of_unit', 'is_idempotent', 'is_integer', 'is_integral', 'is_irreducible', 'is_nilpotent', 'is_norm', 'is_one', 'is_perfect_power', 'is_power', 'is_power_of', 'is_prime', 'is_prime_power', 'is_pseudoprime', 'is_square', 'is_squarefree', 'is_unit', 'is_zero', 'isqrt', 'jacobi', 'kronecker', 'lcm', 'leading_coefficient', 'list', 'log', 'mod', 'multifactorial', 'multiplicative_order', 'n', 'nbits', 'ndigits', 'next_prime', 'next_probable_prime', 'nth_root', 'numerator', 'numerical_approx', 'odd_part', 'ord', 'order', 'ordinal_str', 'parent', 'perfect_power', 'popcount', 'powermod', 'powermodm_ui', 'prime_divisors', 'prime_factors', 'prime_to_m_part', 'quo_rem', 'radical', 'rational_reconstruction', 'real', 'rename', 'reset_name', 'save', 'sign', 'sqrt', 'sqrt_approx', 'sqrtrem', 'squarefree_part', 'str', 'subs', 'substitute', 'support', 'test_bit', 'trailing_zero_bits', 'trial_division', 'val_unit', 'valuation', 'version', 'xgcd']
**********************************************************************
1 item had failures:
   2 of   3 in sage.structure.element.Element.__dir__
    [433 tests, 2 failures, 5.28 s]
----------------------------------------------------------------------
sage -t --long src/sage/structure/element.pyx  # 2 doctests failed
----------------------------------------------------------------------
Total time for all tests: 5.5 seconds
    cpu time: 1.7 seconds
    cumulative wall time: 5.3 seconds
ppurka commented 10 years ago
comment:12

Also, in rational.pyx, you should simply add the line

is_integer = is_integral

inside the class Rational. This should be done instead of the function definition you made.

Edit: Sorry, it looks like I myself wrote the comment:4 ambiguously. What I meant was to use the above line if is_integral is already defined, and only otherwise use the function definition.

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

Changed commit from 85f8025 to 9408464

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

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

9408464Made changes in element.pyx to pass the doc-test.
3shv commented 10 years ago
comment:14

Hi Punarbasu,

I've incorporated the suggested changes.The doc-tests

./sage -t long src/sage/rings 
./sage -t long src/sage/symbolic

have passed.Please review again and give suggestions.Thanks!

ppurka commented 10 years ago
comment:15

Looks good to me now. Thanks! :)

ppurka commented 10 years ago

Author: Rajesh Veeranki

ppurka commented 10 years ago

Reviewer: Punarbasu Purkayastha

jdemeyer commented 10 years ago
comment:16

There is some mis-formatting. There should be an empty line after

EXAMPLES::

and before

def is_integer(self):
jdemeyer commented 10 years ago
comment:17

Also, it is discouraged to start doctests with

"""Some text on the first line
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

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

379511dMade formatting changes
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

Changed commit from 9408464 to 379511d

ppurka commented 10 years ago

Changed branch from u/Rajesh_Veeranki/ticket/15500 to u/ppurka/ticket/15500

ppurka commented 10 years ago
comment:21

Didn't notice the update by Rajesh several weeks ago. Merged 6.2.beta8.


New commits:

4d00b30Merge branch 'develop' into ticket/15500
ppurka commented 10 years ago

Changed commit from 379511d to 4d00b30

ppurka commented 10 years ago

Changed reviewer from Punarbasu Purkayastha to Punarbasu Purkayastha, Jeroen Demeyer

vbraun commented 10 years ago

Changed branch from u/ppurka/ticket/15500 to 4d00b30