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

Remove old notebook files #11409

Closed
jdemeyer opened this issue May 31, 2011 · 66 comments
Closed

Remove old notebook files #11409

jdemeyer opened this issue May 31, 2011 · 66 comments

Comments

@jdemeyer
Copy link

The old Sage notebook (as part of the Sage library) has been superseded by the sagenb spkg. The following files are from the old notebook and should be removed:

  • $SAGE_ROOT/devel/sage/sage/server/introspect.py
  • $SAGE_ROOT/devel/sage/sage/server/notebook/*
  • $SAGE_ROOT/devel/sage/sage/server/simple/*

The following files are duplicated in sagenb but contain some functionality which is not specific to the notebook. At the moment no effort is made to dis-entangle these (but I have made them more uniform)

  • $SAGE_ROOT/devel/sage/sage/server/misc.py
  • $SAGE_ROOT/devel/sage/sage/server/support.py

Apply:

  1. cd sage/server && hg remove introspect.py notebook simple && hg commit -m "Trac #11409: Remove old notebook files" # I prefer this shell command to a patch because there are no chances of a merge conflict
  2. attachment: trac_11409-rebased-v2.patch
  3. attachment: 11409_doctest.patch

After applying the patches, running sdist, and then make from scratch, I get the following results:

$ cd $SAGE_ROOT
$ grep -e 'server.simple' -e 'server.notebook' -e 'server.introspect' -R .
Binary file ./devel/sage/.hg/store/data/doc/en/reference/notebook.rst.i matches
Binary file ./devel/sage/.hg/store/data/_m_a_n_i_f_e_s_t.in.i matches
Binary file ./devel/sage/.hg/store/00manifest.d matches
Binary file ./devel/sage-main/.hg/store/data/doc/en/reference/notebook.rst.i matches
Binary file ./devel/sage-main/.hg/store/data/_m_a_n_i_f_e_s_t.in.i matches
Binary file ./devel/sage-main/.hg/store/00manifest.d matches
./local/share/texmf/tex/generic/sagetex/remote-sagetex.dtx:% in \texttt{sage/server/simple/twist.py}.

This proves that the deleted files are no longer referenced (expect for a comment in SageTeX, this has been reported to Dan Drake).


See also #9237, #9552.

Depends on #13794
Depends on #14330

CC: @kiwifb @williamstein @kini

Component: notebook

Author: Jeroen Demeyer

Reviewer: Frédéric Chapoton, Punarbasu Purkayastha

Merged: sage-5.13.beta2

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

@jdemeyer

This comment has been minimized.

@jdemeyer

This comment has been minimized.

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link
Author

Author: Jeroen Demeyer

@jdemeyer
Copy link
Author

jdemeyer commented Jun 7, 2011

Patch for local/bin

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link
Author

jdemeyer commented Jun 7, 2011

comment:5

Attachment: 11409_scripts.patch.gz

@jdemeyer

This comment has been minimized.

@jdemeyer

This comment has been minimized.

@jdemeyer

This comment has been minimized.

@jdemeyer

This comment has been minimized.

@jdemeyer

This comment has been minimized.

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link
Author

jdemeyer commented Jun 9, 2011

Patch for SAGENB

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link
Author

comment:12

Attachment: 11409_sagenb.patch.gz

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link
Author

comment:13

After applying the patches, running sdist, and then make from scratch, I get the following results:

$ cd $SAGE_ROOT
$ grep -e 'server.simple' -e 'server.notebook' -e 'server.introspect' -R .
Binary file ./devel/sage/.hg/store/data/doc/en/reference/notebook.rst.i matches
Binary file ./devel/sage/.hg/store/data/_m_a_n_i_f_e_s_t.in.i matches
Binary file ./devel/sage/.hg/store/00manifest.d matches
Binary file ./devel/sage-main/.hg/store/data/doc/en/reference/notebook.rst.i matches
Binary file ./devel/sage-main/.hg/store/data/_m_a_n_i_f_e_s_t.in.i matches
Binary file ./devel/sage-main/.hg/store/00manifest.d matches
./local/share/texmf/tex/generic/sagetex/remote-sagetex.dtx:% in \texttt{sage/server/simple/twist.py}.

This proves that the deleted files are no longer referenced (expect for a comment in SageTeX, this has been reported to Dan Drake).

@kcrisman
Copy link
Member

comment:15

Just as a comment, William made some reference at Sage Days 31 to a deprecation period and that these files were needed to enable people to migrate to the new notebook (not the really new flask notebook, nor the old new notebook, but the medium new separate sagenb notebook, I think). I don't know how accurate that is, but thought I should mention it.

@jdemeyer
Copy link
Author

comment:16

Replying to @kcrisman:

Just as a comment, William made some reference at Sage Days 31 to a deprecation period and that these files were needed to enable people to migrate to the new notebook (not the really new flask notebook, nor the old new notebook, but the medium new separate sagenb notebook, I think).

How long should we wait then? sagenb has existed at least since sage-4.2, released 25 october 2009 (almost 2 years ago). I think that is sufficient time.

@kcrisman
Copy link
Member

comment:17

That long, eh?

Anyway, since he mentioned it, I figure it would be worth asking him, since he mentioned it. Being a BDFL has its privileges :)

To be fair, I don't feel like I can review this in any case; it would have to be someone pretty familiar with the mechanics of the change to sagenb. But it would be really nice to take care of this, you are right.

@jdemeyer

This comment has been minimized.

@jdemeyer jdemeyer assigned jdemeyer and unassigned jasongrout Jul 24, 2011
@jdemeyer jdemeyer added this to the sage-5.12 milestone Aug 13, 2013
@fchapoton
Copy link
Contributor

rebased on 5.12.beta4

@fchapoton

This comment has been minimized.

@fchapoton
Copy link
Contributor

comment:41

Attachment: trac_11409-rebased-v2.patch.gz

I have rebased the patch on 5.12.beta4

I had also to put back the import of alarm and cancel_alarm in server/misc.py

for the bot: apply only trac_11409-rebased-v2.patch

@fchapoton
Copy link
Contributor

comment:42

apply only trac_11409-rebased-v2.patch

@fchapoton
Copy link
Contributor

comment:43

hello, the bot is green, so there is an opportunity to review this now !

@ppurka
Copy link
Member

ppurka commented Oct 28, 2013

Reviewer: Punarbasu Purkayastha

@ppurka
Copy link
Member

ppurka commented Oct 28, 2013

comment:44

This looks fine to me. Irrespective of what the bot says, it applies just fine to 5.13.beta1. I tested 3D plots, 2D plots, debug() functionality, and interacts, and they are all working.

@jdemeyer
Copy link
Author

Attachment: 11409_doctest.patch.gz

@jdemeyer

This comment has been minimized.

@ppurka
Copy link
Member

ppurka commented Oct 29, 2013

comment:46

Why didn't the patchbot catch this error earlier? I had only a laptop to run all doctests, so I was expecting the patchbot to report any such problems.

@jdemeyer
Copy link
Author

comment:47

Replying to @ppurka:

Why didn't the patchbot catch this error earlier? I had only a laptop to run all doctests, so I was expecting the patchbot to report any such problems.

I guess the patchbot doesn't understand the "apply" instruction

cd sage/server && hg remove introspect.py notebook simple && hg commit -m "Trac #11409: Remove old notebook files

@ppurka
Copy link
Member

ppurka commented Oct 29, 2013

comment:48

Sorry, the new patch actually introduces the doctest failure. I still get the old result in my copy of 5.13.beta1.

@jdemeyer
Copy link
Author

comment:49

Replying to @ppurka:

I still get the old result in my copy of 5.13.beta1.

Did you actually remove the old notebook files?

@ppurka
Copy link
Member

ppurka commented Oct 29, 2013

comment:50

Replying to @jdemeyer:

Did you actually remove the old notebook files?

Thanks! I rechecked everything in the notebook after trying this. It works fine, and also doctests fine.

@ppurka
Copy link
Member

ppurka commented Oct 29, 2013

Changed reviewer from Punarbasu Purkayastha to Frédéric Chapoton, Punarbasu Purkayastha

@jdemeyer
Copy link
Author

Merged: sage-5.13.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

9 participants