Opened 10 years ago

Closed 10 years ago

#14203 closed enhancement (fixed)

Move DOCTEST_MODE to doctesting framework

Reported by: Jeroen Demeyer Owned by: David Roe
Priority: major Milestone: sage-5.10
Component: doctest framework Keywords:
Cc: John Palmieri Merged in: sage-5.10.beta1
Authors: John Palmieri Reviewers: Volker Braun
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #12415 Stopgaps:

Status badges

Description

Move the DOCTEST_MODE setting to the doctesting framework, for example in the file sage/doctest/all.py.

Attachments (1)

trac_14203_doctest.patch (6.0 KB) - added by John Palmieri 10 years ago.
initial patch

Download all attachments as: .zip

Change History (17)

comment:1 Changed 10 years ago by Jeroen Demeyer

Dependencies: #12415

comment:2 Changed 10 years ago by John Palmieri

Cc: John Palmieri added

comment:3 Changed 10 years ago by John Palmieri

This patch puts DOCTEST_MODE in sage/doctest/__init__.py. It also removes it entirely from sage/plot/plot.py, since it wasn't used there at all. Should we leave it there for backwards compatibility with optional/experimental packages and third party code?

At some point, we might do the same with EMBEDDED_MODE, but this will also require patches to sagenb, and therefore is more annoying.

comment:4 in reply to:  3 ; Changed 10 years ago by David Roe

Replying to jhpalmieri:

This patch puts DOCTEST_MODE in sage/doctest/__init__.py. It also removes it entirely from sage/plot/plot.py, since it wasn't used there at all. Should we leave it there for backwards compatibility with optional/experimental packages and third party code?

We need a combined lazy_import/deprecate for this situation. :-)

At some point, we might do the same with EMBEDDED_MODE, but this will also require patches to sagenb, and therefore is more annoying.

Where would EMBEDDED_MODE move to?

comment:5 in reply to:  4 Changed 10 years ago by John Palmieri

Replying to roed:

Where would EMBEDDED_MODE move to?

Maybe somewhere in sagenb?

comment:6 in reply to:  4 Changed 10 years ago by David Roe

Replying to roed:

We need a combined lazy_import/deprecate for this situation. :-)

I decided to write something: see #14275.

comment:7 in reply to:  3 Changed 10 years ago by Jeroen Demeyer

Replying to jhpalmieri:

At some point, we might do the same with EMBEDDED_MODE, but this will also require patches to sagenb, and therefore is more annoying.

And rename EMBEDDED_MODE to HTML_MODE or something else which describes the usage better.

And EMBEDDED_MODE is actually independently defined in 6 different modules:

devel/sagenb/sagenb/misc/support.py
devel/sage/sage/misc/pager.py
devel/sage/sage/misc/sageinspect.py
devel/sage/sage/misc/latex.py
devel/sage/sage/plot/plot.py
devel/sage/sage/server/support.py

comment:8 Changed 10 years ago by John Palmieri

Is NOTEBOOK_MODE a good name?

Maybe we should import it from sagenb.misc.support for now, and then rename it later. Or we could do this once somewhere in the Sage library:

try:
    from sagenb.misc.support import NOTEBOOK_MODE
except ImportError:
    from sagenb.misc.support import EMBEDDED_MODE as NOTEBOOK_MODE

Then there can be an independent sagenb patch eventually.

comment:9 Changed 10 years ago by David Roe

Component: doctestdoctest framework
Owner: changed from Minh Van Nguyen to David Roe

comment:10 Changed 10 years ago by Jeroen Demeyer

Is this ticket ready for review?

comment:11 Changed 10 years ago by John Palmieri

Status: newneeds_review

Sure, why not?

Changed 10 years ago by John Palmieri

Attachment: trac_14203_doctest.patch added

initial patch

comment:12 Changed 10 years ago by John Palmieri

Rebased to 5.9.beta2.

comment:13 Changed 10 years ago by Volker Braun

Authors: John Palmieri
Milestone: sage-5.9sage-5.10
Reviewers: Volker Braun
Status: needs_reviewpositive_review

Looks good to me!

comment:14 Changed 10 years ago by Karl-Dieter Crisman

Umm... what about the #14275 situation? I don't think this would mess things up as much as moving EMBEDDED_MODE but perhaps it's still unwise to do without this...

comment:15 Changed 10 years ago by Volker Braun

I don't agree with #14275, for starters. It would be a mistake to make any guarantees about the implementation of the doctesting framework. If your code depends on the location of DOCTEST_MODE then your code is broken.

comment:16 Changed 10 years ago by Jeroen Demeyer

Merged in: sage-5.10.beta1
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.