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

Add tests in the category of euclidean domains #15191

Closed
saraedum opened this issue Sep 13, 2013 · 33 comments
Closed

Add tests in the category of euclidean domains #15191

saraedum opened this issue Sep 13, 2013 · 33 comments

Comments

@saraedum
Copy link
Member

Currently, there are no generic tests in the category of euclidean domain which test the correctness of the quotient with remainder.

Depends on #14482
Depends on #15192

Component: categories

Keywords: sd59

Author: Julian Rueth

Branch/Commit: 6261466

Reviewer: Peter Bruin, Travis Scrimshaw, Frédéric Chapoton

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

@saraedum saraedum added this to the sage-5.12 milestone Sep 13, 2013
@saraedum
Copy link
Member Author

Branch: u/saraedum/ticket/15191

@saraedum
Copy link
Member Author

Dependencies: #14482, #15192

@saraedum
Copy link
Member Author

Author: Julian Rueth

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 13, 2013

Commit: 654b8a8

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 13, 2013

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

[changeset:654b8a8]Implement a non-trivial some_elements() for polynomial rings

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.0, sage-6.1 Dec 17, 2013
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014
@pjbruin
Copy link
Contributor

pjbruin commented Apr 26, 2014

Changed branch from u/saraedum/ticket/15191 to u/pbruin/15191-EuclideanDomains_tests

@pjbruin
Copy link
Contributor

pjbruin commented Apr 26, 2014

comment:7

resolved merge conflicts

@pjbruin
Copy link
Contributor

pjbruin commented Apr 26, 2014

Changed commit from 654b8a8 to 9c44bec

@pjbruin
Copy link
Contributor

pjbruin commented Apr 26, 2014

Work Issues: fix doctest failures

@pjbruin
Copy link
Contributor

pjbruin commented Apr 26, 2014

comment:8

After resolving the conflicts there are various doctest failures:

sage -t --long src/sage/rings/padics/padic_base_leaves.py  # 3 doctests failed
sage -t --long src/sage/rings/polynomial/polynomial_ring.py  # 1 doctest failed
sage -t --long src/doc/en/thematic_tutorials/coercion_and_categories.rst  # 3 doctests failed
sage -t --long src/doc/en/thematic_tutorials/tutorial-objects-and-classes.rst  # 2 doctests failed

Some of these are due to the testsuite running euclidean_degree() on random elements; these may be zero, which causes

NotImplementedError: euclidean degree not defined for the zero element

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@saraedum
Copy link
Member Author

Changed keywords from none to sd59

@saraedum
Copy link
Member Author

Changed branch from u/pbruin/15191-EuclideanDomains_tests to u/saraedum/ticket/15191

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 29, 2014

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

f60c865Fixed documentation doctests

@tscrim
Copy link
Collaborator

tscrim commented Jun 30, 2014

Changed commit from f60c865 to ce64f71

@tscrim
Copy link
Collaborator

tscrim commented Jun 30, 2014

Changed work issues from fix doctest failures to none

@saraedum
Copy link
Member Author

comment:14

Like many inexact rings, the p-adics have zerodivisors. Not in theory of course, but in practice (i.e. the p-adics are an integral domain but typically implemented with finite precision which leads to zero-divisors). That's why I put the continue there if a or a*b are zero. I had put a remark in the commit message but not in the code (which was probably not very smart).

The check you put there is valid for exact rings however. As you say it should probably not be in the category of euclidean domains. Would you mind moving this to a different ticket?

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 1, 2014

Changed commit from ce64f71 to 75fa2db

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 1, 2014

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

75fa2dbMade a note about zero divisors.

@tscrim
Copy link
Collaborator

tscrim commented Jul 1, 2014

comment:16

Okay, I'v reverted the change and created #16596 for the zero divisors tests (I'll post a branch there shortly).

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 30, 2014

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

dfb049eMerge branch 'public/ticket/euclidean_domains_tests-15191' of ssh://trac.sagemath.org:22/sage into 15191
7f30ddctrac #15191 change bad syntax of 2 input blocks

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 30, 2014

Changed commit from 75fa2db to 7f30ddc

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 1, 2014

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

248dfbcSome more review tweaks and fixes.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 1, 2014

Changed commit from 7f30ddc to 248dfbc

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 1, 2014

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

6261466Stupid typo...

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 1, 2014

Changed commit from 248dfbc to 6261466

@saraedum
Copy link
Member Author

saraedum commented Aug 1, 2014

comment:21

Thanks for your review patches. What is the status of this ticket now? Do you want a review of your latest changes?

@tscrim
Copy link
Collaborator

tscrim commented Aug 1, 2014

comment:22

Yes please (and Frédéric's too). If you're happy with the changes, then I believe it's a positive review. (Also I'd appreciate a review of #16596 too.)

@tscrim
Copy link
Collaborator

tscrim commented Aug 1, 2014

Changed reviewer from Peter Bruin, Travis Scrimshaw to Peter Bruin, Travis Scrimshaw, Frédéric Chapoton

@vbraun
Copy link
Member

vbraun commented Aug 4, 2014

Changed branch from public/ticket/euclidean_domains_tests-15191 to 6261466

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