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

Regression in .divides() for generic polynomials #27064

Closed
defeo opened this issue Jan 15, 2019 · 13 comments
Closed

Regression in .divides() for generic polynomials #27064

defeo opened this issue Jan 15, 2019 · 13 comments

Comments

@defeo
Copy link
Member

defeo commented Jan 15, 2019

This used to work prior to #19171:

sage: R.<t> = GF(5)[]
sage: K = R.fraction_field()
sage: A.<x> = K[]
sage: x.divides(x)
True

Now it gives

---------------------------------------------------------------------------                                                            
AttributeError                            Traceback (most recent call last)                                                            
<ipython-input-14-cd2656325e62> in <module>()                                                                                          
----> 1 x.divides(x)                                                                                                                   
                                                                                                                                       
/data/dfl/sage/local/lib/python2.7/site-packages/sage/structure/element.pyx in sage.structure.element.coerce_binop.new_method (build/cy
thonized/sage/structure/element.c:28683)()                                                                                             
   4269     def new_method(self, other, *args, **kwargs):                                                                              
   4270         if have_same_parent(self, other):                                                                                      
-> 4271             return method(self, other, *args, **kwargs)                                                                        
   4272         else:                                                                                                                  
   4273             a, b = coercion_model.canonical_coercion(self, other)                                                              
                                                                                                                                       
/data/dfl/sage/local/lib/python2.7/site-packages/sage/rings/polynomial/polynomial_element.pyx in sage.rings.polynomial.polynomial_elem$
nt.Polynomial.divides (build/cythonized/sage/rings/polynomial/polynomial_element.c:93668)()
  10088             return False  
  10089                                                                                                                                
> 10090         if not self.leading_coefficient().divides(p.leading_coefficient()):              
  10091             return False
  10092

/data/dfl/sage/local/lib/python2.7/site-packages/sage/structure/element.pyx in sage.structure.element.Element.__getattr__ (build/cytho$
ized/sage/structure/element.c:4550)()
    491             AttributeError: 'LeftZeroSemigroup_with_category.element_class' object has no attribute 'blah_blah'
    492         """
--> 493         return self.getattr_from_category(name)
    494
    495     cdef getattr_from_category(self, name):

/data/dfl/sage/local/lib/python2.7/site-packages/sage/structure/element.pyx in sage.structure.element.Element.getattr_from_category (bu
ild/cythonized/sage/structure/element.c:4659)()
    504         else:
    505             cls = P._abstract_element_class
--> 506         return getattr_from_other_class(self, cls, name)
    507
    508     def __dir__(self):

/data/dfl/sage/local/lib/python2.7/site-packages/sage/cpython/getattr.pyx in sage.cpython.getattr.getattr_from_other_class (build/cytho
nized/sage/cpython/getattr.c:2536)()
    392         dummy_error_message.cls = type(self)
    393         dummy_error_message.name = name
--> 394         raise AttributeError(dummy_error_message)
    395     attribute = <object>attr
    396     # Check for a descriptor (__get__ in Python)

AttributeError: 'sage.rings.fraction_field_FpT.FpTElement' object has no attribute 'divides'

This is failing because the fraction field elements have no .divides() method.

CC: @bgrenet

Component: basic arithmetic

Keywords: polynomials division

Author: Luca De Feo

Branch/Commit: 4c14569

Reviewer: Jeroen Demeyer

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

@defeo defeo added this to the sage-8.6 milestone Jan 15, 2019
@defeo
Copy link
Member Author

defeo commented Jan 15, 2019

comment:1

I guess the solution is to add a trivial .divides() method to fields? I am not sure why there is no such method already.

@jdemeyer

This comment has been minimized.

@jdemeyer jdemeyer changed the title Regression in .divides() for polynomials Regression in .divides() for generic polynomials Jan 15, 2019
@embray
Copy link
Contributor

embray commented Jan 15, 2019

comment:3

Retarging tickets optimistically to the next milestone. If you are responsible for this ticket (either its reporter or owner) and don't believe you are likely to complete this ticket before the next release (8.7) please retarget this ticket's milestone to sage-pending or sage-wishlist.

@embray embray modified the milestones: sage-8.6, sage-8.7 Jan 15, 2019
@bgrenet
Copy link
Contributor

bgrenet commented Jan 16, 2019

comment:4

Two solutions (seem to) work:

  • Either add a trivial divides method to the ElementMethods of the category Fields;
  • Make FpTElement inherits from FieldElement rather than RingElement.

Actually, I do not know why this class FpTElement inherits from RingElement rather than FieldElement. It would even make more sense for it to inherit from FractionFieldElement I would say.

@defeo
Copy link
Member Author

defeo commented Jan 16, 2019

comment:5

FractionFieldElement is a Python class, I don't think it can inherit from it.

But I tested your second solution and it works. I'm preparing the commit.

@defeo
Copy link
Member Author

defeo commented Jan 16, 2019

@defeo
Copy link
Member Author

defeo commented Jan 16, 2019

Commit: 4c14569

@defeo
Copy link
Member Author

defeo commented Jan 16, 2019

New commits:

4c14569Made elements of GF(p)(T) inherit from field (instead of ring)

@jdemeyer
Copy link
Contributor

Author: Luca De Feo

@jdemeyer
Copy link
Contributor

Reviewer: Jeroen Demeyer

@jdemeyer
Copy link
Contributor

comment:8

Fine for me if it passes doctests.

@defeo
Copy link
Member Author

defeo commented Jan 16, 2019

comment:9

All doctests (--all --long) pass for me.

@vbraun
Copy link
Member

vbraun commented Jan 26, 2019

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

5 participants