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

Move DOCTEST_MODE to doctesting framework #14203

Closed
jdemeyer opened this issue Feb 28, 2013 · 17 comments
Closed

Move DOCTEST_MODE to doctesting framework #14203

jdemeyer opened this issue Feb 28, 2013 · 17 comments

Comments

@jdemeyer
Copy link
Contributor

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

Depends on #12415

CC: @jhpalmieri

Component: doctest framework

Author: John Palmieri

Reviewer: Volker Braun

Merged: sage-5.10.beta1

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

@jdemeyer
Copy link
Contributor Author

Dependencies: #12415

@jhpalmieri
Copy link
Member

comment:3

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.

@roed314
Copy link
Contributor

roed314 commented Mar 14, 2013

comment:4

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?

@jhpalmieri
Copy link
Member

comment:5

Replying to @roed314:

Where would EMBEDDED_MODE move to?

Maybe somewhere in sagenb?

@roed314
Copy link
Contributor

roed314 commented Mar 14, 2013

comment:6

Replying to @roed314:

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

I decided to write something: see #14275.

@jdemeyer
Copy link
Contributor Author

comment:7

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

@jhpalmieri
Copy link
Member

comment:8

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.

@jdemeyer
Copy link
Contributor Author

jdemeyer commented Apr 3, 2013

comment:10

Is this ticket ready for review?

@jhpalmieri
Copy link
Member

comment:11

Sure, why not?

@jhpalmieri
Copy link
Member

initial patch

@jhpalmieri
Copy link
Member

comment:12

Attachment: trac_14203_doctest.patch.gz

Rebased to 5.9.beta2.

@vbraun
Copy link
Member

vbraun commented Apr 20, 2013

Author: John Palmieri

@vbraun
Copy link
Member

vbraun commented Apr 20, 2013

Reviewer: Volker Braun

@vbraun
Copy link
Member

vbraun commented Apr 20, 2013

comment:13

Looks good to me!

@kcrisman
Copy link
Member

comment:14

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

@vbraun
Copy link
Member

vbraun commented Apr 22, 2013

comment:15

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.

@jdemeyer
Copy link
Contributor Author

Merged: sage-5.10.beta1

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

5 participants