Opened 11 years ago
Closed 8 years ago
#383 closed defect (fixed)
quo_rem in the polynomial rings does not use canonical coercion
Reported by: | jbmohler | Owned by: | somebody |
---|---|---|---|
Priority: | minor | Milestone: | sage-4.3.1 |
Component: | basic arithmetic | Keywords: | |
Cc: | Merged in: | sage-4.3.1.rc2 | |
Authors: | Robert Bradshaw | Reviewers: | William Stein |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
I'm looking at the polynomial function quo_rem and I see that it does it's own coercion manually. This feels a little wrong to me. I think it should go through the standard coercion routines. Here's a "bug" that results:
sage: x=ZZ['x'].0 sage: y=QQ['x'].0 sage: (y+1).quo_rem(1/2*x) (2, 1) sage: (x+1).quo_rem(1/2*y) ... <type 'exceptions.TypeError'>: no coercion of this rational to integer
The bug is that I don't see why these two things are treated substantially differently. The reason I found this is because the simple "TypeError?" exception did not provide the usual message about parents being mis-matched -- I think this is a bug in itself
The fix for all that is to make the quo_rem stuff use canonical coercion model.
All of the quo_rem instances in sage/rings/polynomial/polynomial_element_generic.py suffer from some sort of coercion impropriety.
Attachments (3)
Change History (13)
comment:1 Changed 11 years ago by
- Milestone set to Sage-2.10
comment:2 Changed 8 years ago by
- Report Upstream set to N/A
- Status changed from new to needs_review
comment:3 Changed 8 years ago by
Changed 8 years ago by
comment:4 Changed 8 years ago by
Oops. Thanks, fixed.
comment:5 Changed 8 years ago by
I read the code. Looks AWESOME!
It appears to expose numerous issues:
sage -t devel/sage/sage/rings ... The following tests failed: sage -t devel/sage/sage/rings/finite_field_element.py # 3 doctests failed sage -t devel/sage/sage/rings/tests.py # 1 doctests failed sage -t devel/sage/sage/rings/finite_field_ext_pari.py # 1 doctests failed sage -t devel/sage/sage/rings/fraction_field_element.pyx # 1 doctests failed sage -t devel/sage/sage/rings/polynomial/polynomial_integer_dense_flint.pyx # 1 doctests failed sage -t devel/sage/sage/rings/residue_field.pyx # 3 doctests failed sage -t devel/sage/sage/rings/polynomial/polynomial_zmod_flint.pyx # 5 doctests failed sage -t devel/sage/sage/rings/number_field/number_field_ideal.py # 2 doctests failed ---------------------------------------------------------------------- Total time for all tests: 192.3 seconds [1]+ Done ./sage -tp 10 devel/sage/sage/rings > 383.out wstein@boxen:~/build/sage-4.3.1.rc0$ pwd /home/wstein/build/sage-4.3.1.rc0 wstein@boxen:~/build/sage-4.3.1.rc0$ ls 383.out 6207.out~ data dist install.log local README.txt sage-python spkg tmp 6207.out COPYING.txt devel examples ipython makefile sage sage-README-osx.txt test.log wstein@boxen:~/build/sage-4.3.1.rc0$ pwd /home/wstein/build/sage-4.3.1.rc0 wstein@boxen:~/build/sage-4.3.1.rc0$
Changed 8 years ago by
comment:6 Changed 8 years ago by
OK, I've fixed all the above doctests issues.
comment:7 Changed 8 years ago by
- Status changed from needs_review to needs_work
Ut oh:
---------------------------------------------------------------------- The following tests failed: sage -t devel/sage/sage/crypto/classical.py # 14 doctests failed sage -t devel/sage/sage/modular/etaproducts.py # 24 doctests failed sage -t devel/sage/sage/structure/element.pyx # 1 doctests failed sage -t devel/sage/sage/libs/pari/gen.pyx # Segfault sage -t devel/sage/sage/modular/arithgroup/arithgroup_generic.py # 4 doctests failed sage -t devel/sage/sage/modular/arithgroup/congroup_gamma0.py # 2 doctests failed sage -t devel/sage/sage/quadratic_forms/quadratic_form__split_local_covering.py # 2 doctests failed sage -t devel/sage/sage/modular/cusps.py # 1 doctests failed ---------------------------------------------------------------------- Total time for all tests: 478.6 seconds wstein@boxen:~/build/sage-4.3.1.rc0-boxen-x86_64-Linux$
Changed 8 years ago by
comment:8 Changed 8 years ago by
- Status changed from needs_work to needs_review
OK, I've doctested the entire sage library this time.
comment:9 Changed 8 years ago by
- Status changed from needs_review to positive_review
comment:10 Changed 8 years ago by
- Merged in set to sage-4.3.1.rc2
- Resolution set to fixed
- Reviewers set to William Stein
- Status changed from positive_review to closed
Typo: arithmatic --> arithmetic