Opened 7 years ago

Closed 7 years ago

#14203 closed enhancement (fixed)

Move DOCTEST_MODE to doctesting framework

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

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 jhpalmieri 7 years ago.
initial patch

Download all attachments as: .zip

Change History (17)

comment:1 Changed 7 years ago by jdemeyer

  • Dependencies set to #12415

comment:2 Changed 7 years ago by jhpalmieri

  • Cc jhpalmieri added

comment:3 follow-ups: Changed 7 years ago by 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?

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 ; follow-ups: Changed 7 years ago by roed

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 7 years ago by jhpalmieri

Replying to roed:

Where would EMBEDDED_MODE move to?

Maybe somewhere in sagenb?

comment:6 in reply to: ↑ 4 Changed 7 years ago by roed

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 7 years ago by jdemeyer

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 7 years ago by jhpalmieri

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 7 years ago by roed

  • Component changed from doctest to doctest framework
  • Owner changed from mvngu to roed

comment:10 Changed 7 years ago by jdemeyer

Is this ticket ready for review?

comment:11 Changed 7 years ago by jhpalmieri

  • Status changed from new to needs_review

Sure, why not?

Changed 7 years ago by jhpalmieri

initial patch

comment:12 Changed 7 years ago by jhpalmieri

Rebased to 5.9.beta2.

comment:13 Changed 7 years ago by vbraun

  • Authors set to John Palmieri
  • Milestone changed from sage-5.9 to sage-5.10
  • Reviewers set to Volker Braun
  • Status changed from needs_review to positive_review

Looks good to me!

comment:14 Changed 7 years ago by kcrisman

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 7 years ago by vbraun

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 7 years ago by jdemeyer

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