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

change "#optional - jones_database" to "#optional - database_jones_numfield", and several other similar renames needed #12335

Closed
williamstein opened this issue Jan 21, 2012 · 16 comments

Comments

@williamstein
Copy link
Contributor

The doctests in devel/sage/sage/databases/jones.py are marked "#optional - jones_database". These test the optional package database_jones_numfield-v4.spkg, so they should be labeled "#optional - database_jones_numfield".

For the record, this was my fault (William Stein) in #4588. Sorry.

Apply only attachment: 12335.patchThe Macaulay2 fixes are basically irrelevant until that spkg is fixed.

Component: doctest coverage

Keywords: sd40.5

Author: Dan Drake

Reviewer: Benjamin Jones

Merged: sage-5.1.beta2

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

@williamstein
Copy link
Contributor Author

comment:1

Also, similarly rename stein_watkins_database:

databases/stein_watkins.py:51:    sage: C = d.next()                                   # optional - stein_watkins_database
databases/stein_watkins.py:52:    sage: C                                              # optional - stein_watkins_database
databases/stein_watkins.py:54:    sage: d.next()                                       # optional - stein_watkins_database
databases/stein_watkins.py:56:    sage: d.next()                                       # optional - stein_watkins_database

@williamstein
Copy link
Contributor Author

comment:2

Similar for Macaulay2, where the markings are all over the place! It should be "# optional - macaulay2" everywhere, but M2, nothing, and macaulay2 appear in various places:

sage: search_src('macaulay2')                                                         
...
modules/free_module.py:1936:            sage: macaulay2(R) # optional
...
rings/polynomial/multi_polynomial_element.py:298:#            sage: macaulay2(R)                      # optional, requires M2
...

@williamstein williamstein changed the title change "#optional - jones_database" to "#optional - database_jones_numfield" change "#optional - jones_database" to "#optional - database_jones_numfield", and several other similar renames needed Jan 21, 2012
@dandrake
Copy link
Contributor

for databases

@dandrake
Copy link
Contributor

Author: Dan Drake

@dandrake
Copy link
Contributor

comment:3

Attachment: 12335.patch.gz

This fixes the Jones and Stein-Watkins files. I'll work on another patch for Macaulay2.

@dandrake
Copy link
Contributor

Changed keywords from none to sd40.5

@benjaminfjones
Copy link
Contributor

comment:4

For sage/databases/jones.py with the patch applied:

$ ../../sage -t -verbose sage/databases/jones.py
...
21 passed and 0 failed.

$ ../../sage -t -verbose -optional sage/databases/jones.py
...
35 passed and 0 failed.

$ ../../sage -t -verbose -only-optional=database_jones_numfield  sage/databases/jones.py
...
31 passed and 0 failed.

That looks correct to me. I'm testing the stein_watkins database now.

@benjaminfjones
Copy link
Contributor

Reviewer: Benjamin Jones

@benjaminfjones
Copy link
Contributor

comment:5

For sage/databases/stein_watkins.py with the patch applied:

$ ../../sage -t -verbose sage/databases/stein_watkins.py
...
14 passed and 0 failed.

with database_stein_watkins_mini.p0 optional package installed:

$ ../../sage -t -verbose -optional sage/databases/stein_watkins.py
...
37 passed and 0 failed.

$ ../../sage -t -verbose -only-optional=database_stein_watkins sage/databases/stein_watkins.py
...
28 passed and 0 failed.

@dandrake
Copy link
Contributor

Attachment: 12335-macaulay2.patch.gz

@dandrake
Copy link
Contributor

comment:6

The Macaulay2 patch seems intimidating, but it should only change #optional doctest tags and delete trailing whitespace.

@benjaminfjones
Copy link
Contributor

comment:7

The Macaulay2 patch is going to be tough (or impossible) to review until the macaulay spkg can be built. See #11710.

@dandrake

This comment has been minimized.

@dandrake
Copy link
Contributor

comment:8

Replying to @benjaminfjones:

The Macaulay2 patch is going to be tough (or impossible) to review until the macaulay spkg can be built. See #11710.

In that case, let's ignore that patch. Apply only attachment: 12335.patch and review that.

@benjaminfjones
Copy link
Contributor

comment:9

In that case, positive review. Everything looks good.

@jdemeyer
Copy link

jdemeyer commented Jun 2, 2012

Merged: sage-5.1.beta2

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