Improve coverage test for structure/element.pyx

[sagetrac-ejeanvoi]

Improve coverage test for structure/element.pyx

Status as of Sage 6.2.beta4:

SCORE src/sage/structure/element.pyx: 27.5% (42 of 153)

with patch:

SCORE src/sage/structure/element.pyx: 32.7% (50 of 153)

Component: doctest coverage

Author: Emmanuel Jeanvoine, Frédéric Chapoton

Branch/Commit: dbf722e

Reviewer: Nathann Cohen, Jeroen Demeyer

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

[sagetrac-ejeanvoi]

Added some doctests

[KPanComputes]

comment:1

Attachment: trac_10779-doctests.patch.gz

[KPanComputes]

comment:2

Hello!

• Please refer to the conventions page for how to write in ReST markup. For instance, you need a blank line after :: for getting verbatim environment. Also, please note that, a verbatim environment is preceded by ::.

• It'd be nice if you also go ahead and add more doctests. Also, please correct the docstrings formatting whenever you can -- like codifying self, True and such by using two backticks...

• Major: "ERROR: Please add a TestSuite(s).run() doctest."

Hoping to hear from you soon...

Cheers, Kannappan.

[fchapoton]

replaces previous patch

[fchapoton]

comment:5

Attachment: trac_10779-doctests-v2.patch.gz

apply only trac_10779-doctests-v2.patch

[fchapoton]

Commit: c84246e

[fchapoton]

New commits:

c84246e

#10779: Improve coverage test for structure/element.pyx

[fchapoton]

Branch: u/chapoton/10779

[sagetrac-git]

Changed commit from c84246e to db023d4

[sagetrac-git]

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

db023d4

Merge branch 'u/chapoton/10779' of ssh://trac.sagemath.org:22/sage into 10779

[sagetrac-git]

Changed commit from db023d4 to 2e30025

[sagetrac-git]

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

9d72c81	Merge branch 'u/chapoton/10779' of ssh://trac.sagemath.org:22/sage into 10779
9531937	trac #10779 correct failing doctest + more doc changes

[sagetrac-git]

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

54396cd

Merge branch 'u/chapoton/10779' into 6.4

[sagetrac-git]

Changed commit from 9531937 to 54396cd

[nathanncohen]

comment:16

Hello !

This code seems good, but I do not understand the three functions lcm,gcd, and now (with your branch) xgcd in element.pyx. Isn't it more reasonable to import them from sage.ring.arith instead ? (a lazy import if that was the problem)

Nathann

[fchapoton]

comment:17

xgcd was already there. This branch only changes the doc.

[nathanncohen]

comment:18

Oh sorry, I made this mistake by reading the diff file.

Okay let's go.

Nathann

[nathanncohen]

Reviewer: Nathann Cohen

[jdemeyer]

comment:20

It is especially important to
keep this in mind whenever you move a class down from Python to
Cython.

Is this really true? I think Cython is just following the Python convention here...

[jdemeyer]

comment:21

The tests for gcd, lcm and xgcd do not actually test the functions from element.pyx.

[nathanncohen]

comment:22

The tests for gcd, lcm and xgcd do not actually test the functions from element.pyx.

It's true. Do we create a patch to remove them and import the actual ones ? It's probably only to avoid a direct import.

Nathann

[jdemeyer]

Changed branch from u/chapoton/10779 to u/jdemeyer/ticket/10779

[jdemeyer]

Changed reviewer from Nathann Cohen to Nathann Cohen, Jeroen Demeyer

[jdemeyer]

New commits:

dbf722e

Remove gcd, lcm, xgcd from element.pyx

[jdemeyer]

Changed commit from 54396cd to dbf722e

[nathanncohen]

comment:25

Passes all long tests, does the job.

Nathann

[vbraun]

Changed branch from u/jdemeyer/ticket/10779 to dbf722e