Opened 8 years ago

Closed 7 years ago

#12335 closed defect (fixed)

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

Reported by: was Owned by: mvngu
Priority: trivial Milestone: sage-5.1
Component: doctest coverage Keywords: sd40.5
Cc: Merged in: sage-5.1.beta2
Authors: Dan Drake Reviewers: Benjamin Jones
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by ddrake)

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.patch. The Macaulay2 fixes are basically irrelevant until that spkg is fixed.

Attachments (2)

12335.patch (15.7 KB) - added by ddrake 7 years ago.
for databases
12335-macaulay2.patch (283.1 KB) - added by ddrake 7 years ago.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 8 years ago by was

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

comment:2 Changed 8 years ago by was

  • Summary changed from change "#optional - jones_database" to "#optional - database_jones_numfield" to change "#optional - jones_database" to "#optional - database_jones_numfield", and several other similar renames needed

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
...

Changed 7 years ago by ddrake

for databases

comment:3 Changed 7 years ago by ddrake

  • Authors set to Dan Drake
  • Keywords sd40.5 added
  • Status changed from new to needs_review

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

comment:4 Changed 7 years ago by benjaminfjones

  • Reviewers set to Benjamin Jones

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.

comment:5 Changed 7 years ago by benjaminfjones

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.

Changed 7 years ago by ddrake

comment:6 Changed 7 years ago by ddrake

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

comment:7 follow-up: Changed 7 years ago by benjaminfjones

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

comment:8 in reply to: ↑ 7 Changed 7 years ago by ddrake

  • Description modified (diff)

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.

comment:9 Changed 7 years ago by benjaminfjones

  • Status changed from needs_review to positive_review

In that case, positive review. Everything looks good.

comment:10 Changed 7 years ago by jdemeyer

  • Merged in set to sage-5.1.beta2
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.