Opened 9 years ago

Closed 9 years ago

#14186 closed defect (fixed)

coerce_binop errors with keyword arguments

Reported by: lftabera Owned by: robertwb
Priority: major Milestone: sage-6.1
Component: coercion Keywords: coerce_binop, keyword
Cc: Merged in:
Authors: Luis Felipe Tabera Alonso Reviewers: Robert Bradshaw, Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: u/lftabera/ticket/14186 (Commits, GitHub, GitLab) Commit: 0cb642bc59e625e80c6a69e84949ca47dc4e8adc
Dependencies: Stopgaps:

Status badges

Description

On a coerced binary operator

@coerce_binop
__op__(x,y,**kwds)
}}

if y has to be coerced but x does not change its parent, the keyword arguments are not correctly passed.

A real example

{{{
sage: R1=QQ['x,y']
sage: R2=QQ['x,y,z']
sage: f=R1(1)
sage: h=R2(1)
sage: h.gcd(f,algorithm='modular')
TypeError                                 Traceback (most recent call last)
...
TypeError: algorithm algorithm not supported
}}}

Attachments (1)

coer_binop_keywords.patch (2.3 KB) - added by lftabera 9 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 Changed 9 years ago by lftabera

  • Status changed from new to needs_review

It is a one-line fix.

By the way, if a binary method with keyword values that is not decorated

__op__(self, y, method='z')

One can call it with x.__op__(y, 'preferred_method')

But once it is decorated this is not possible and key=value arguments are mandatory. Is this inteded?

Last edited 9 years ago by lftabera (previous) (diff)

comment:2 Changed 9 years ago by lftabera

  • Authors set to Luis Felipe Tabera Alonso

Changed 9 years ago by lftabera

comment:3 Changed 9 years ago by lftabera

Update patch to new coercion framework.

comment:4 Changed 9 years ago by robertwb

  • Reviewers set to Robert Bradshaw
  • Status changed from needs_review to positive_review

Looks good to me.

comment:5 Changed 9 years ago by robertwb

  • Status changed from positive_review to needs_work

comment:6 Changed 9 years ago by robertwb

  • Status changed from needs_work to needs_review

Lets make sure the tests pass though.

comment:7 Changed 9 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:8 Changed 9 years ago by lftabera

Robert,

Could you please take a look to the ticket? It is a one liner, you claimed that looked good and after seven months, patchbot has not complained.

comment:9 Changed 9 years ago by lftabera

  • Branch set to u/lftabera/ticket/14186

comment:10 Changed 9 years ago by git

  • Commit set to 0cb642bc59e625e80c6a69e84949ca47dc4e8adc

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

0cb642bTrac #14186 coerce_binop errors with keyword arguments

comment:11 Changed 9 years ago by tscrim

Looks good to me.

comment:12 Changed 9 years ago by tscrim

  • Status changed from needs_review to positive_review

comment:13 Changed 9 years ago by lftabera

  • Reviewers changed from Robert Bradshaw to Robert Bradshaw, Travis Scrimshaw

Do not forget to add yoursef to the reviewers field

comment:14 Changed 9 years ago by vbraun

  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.