Opened 3 years ago

Closed 3 years ago

#27064 closed defect (fixed)

Regression in .divides() for generic polynomials

Reported by: defeo Owned by:
Priority: major Milestone: sage-8.7
Component: basic arithmetic Keywords: polynomials division
Cc: bruno Merged in:
Authors: Luca De Feo Reviewers: Jeroen Demeyer
Report Upstream: N/A Work issues:
Branch: 4c14569 (Commits, GitHub, GitLab) Commit: 4c145698e9c9d1f1e3e2437e49f9d2da87787e79
Dependencies: Stopgaps:

Status badges

Description (last modified by jdemeyer)

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)

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
   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  
> 10090         if not self.leading_coefficient().divides(p.leading_coefficient()):              
  10091             return False

/data/dfl/sage/local/lib/python2.7/site-packages/sage/structure/element.pyx in sage.structure.element.Element.__getattr__ (build/cytho$
    491             AttributeError: 'LeftZeroSemigroup_with_category.element_class' object has no attribute 'blah_blah'
    492         """
--> 493         return self.getattr_from_category(name)
    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
    504         else:
    505             cls = P._abstract_element_class
--> 506         return getattr_from_other_class(self, cls, name)
    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
    392         dummy_error_message.cls = type(self)
    393 = 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.

Change History (10)

comment:1 Changed 3 years ago by defeo

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

comment:2 Changed 3 years ago by jdemeyer

  • Component changed from PLEASE CHANGE to basic arithmetic
  • Description modified (diff)
  • Summary changed from Regression in .divides() for polynomials to Regression in .divides() for generic polynomials
  • Type changed from PLEASE CHANGE to defect

comment:3 Changed 3 years ago by embray

  • Milestone changed from sage-8.6 to sage-8.7

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.

comment:4 Changed 3 years ago by bruno

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.

comment:5 Changed 3 years ago by defeo

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.

comment:6 Changed 3 years ago by defeo

  • Branch set to u/defeo/regression_in__divides___for_generic_polynomials

comment:7 Changed 3 years ago by defeo

  • Commit set to 4c145698e9c9d1f1e3e2437e49f9d2da87787e79
  • Status changed from new to needs_review

New commits:

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

comment:8 Changed 3 years ago by jdemeyer

  • Authors set to Luca De Feo
  • Reviewers set to Jeroen Demeyer

Fine for me if it passes doctests.

comment:9 Changed 3 years ago by defeo

  • Status changed from needs_review to positive_review

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

comment:10 Changed 3 years ago by vbraun

  • Branch changed from u/defeo/regression_in__divides___for_generic_polynomials to 4c145698e9c9d1f1e3e2437e49f9d2da87787e79
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.