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

is_integer method is missing on integers #15500

Closed
ppurka opened this issue Dec 9, 2013 · 29 comments
Closed

is_integer method is missing on integers #15500

ppurka opened this issue Dec 9, 2013 · 29 comments

Comments

@ppurka
Copy link
Member

ppurka commented Dec 9, 2013

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

@ppurka ppurka added this to the sage-6.1 milestone Dec 9, 2013
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014
@3shv
Copy link

3shv commented Feb 12, 2014

Branch: u/Rajesh_Veeranki/ticket/15500

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 12, 2014

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

4c4826eAdded is_integral() function to integer class

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 12, 2014

Commit: 4c4826e

@ppurka
Copy link
Member Author

ppurka commented Feb 12, 2014

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 12, 2014

Changed commit from 4c4826e to 6ca987d

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 12, 2014

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

6ca987dAdded is_integer() method to integer class

@ppurka
Copy link
Member Author

ppurka commented Feb 13, 2014

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.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 13, 2014

Changed commit from 6ca987d to db07cb1

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 13, 2014

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 13, 2014

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

85f8025Added is_integer() function to Real Number class

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 13, 2014

Changed commit from db07cb1 to 85f8025

@3shv
Copy link

3shv commented Feb 13, 2014

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
Copy link
Member Author

ppurka commented Feb 14, 2014

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
Copy link
Member Author

ppurka commented Feb 14, 2014

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.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 14, 2014

Changed commit from 85f8025 to 9408464

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 14, 2014

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

9408464Made changes in element.pyx to pass the doc-test.

@3shv
Copy link

3shv commented Feb 14, 2014

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
Copy link
Member Author

ppurka commented Feb 14, 2014

comment:15

Looks good to me now. Thanks! :)

@ppurka
Copy link
Member Author

ppurka commented Feb 14, 2014

Author: Rajesh Veeranki

@ppurka
Copy link
Member Author

ppurka commented Feb 14, 2014

Reviewer: Punarbasu Purkayastha

@jdemeyer
Copy link

comment:16

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

EXAMPLES::

and before

def is_integer(self):

@jdemeyer
Copy link

comment:17

Also, it is discouraged to start doctests with

"""Some text on the first line

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 23, 2014

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

379511dMade formatting changes

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 23, 2014

Changed commit from 9408464 to 379511d

@ppurka
Copy link
Member Author

ppurka commented Apr 15, 2014

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

@ppurka
Copy link
Member Author

ppurka commented Apr 15, 2014

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
Copy link
Member Author

ppurka commented Apr 15, 2014

Changed commit from 379511d to 4d00b30

@ppurka
Copy link
Member Author

ppurka commented Apr 15, 2014

Changed reviewer from Punarbasu Purkayastha to Punarbasu Purkayastha, Jeroen Demeyer

@vbraun
Copy link
Member

vbraun commented Apr 16, 2014

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

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

4 participants