Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#15335 closed defect (fixed)

Do not import sagenb at startup

Reported by: chapoton Owned by:
Priority: major Milestone: sage-6.1
Component: misc Keywords: startup
Cc: Merged in:
Authors: Jeroen Demeyer Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: u/jdemeyer/ticket/15335 (Commits) Commit: da0d13387a2a52ec882b6c1d3ccd8c4651f17fdb
Dependencies: Stopgaps:

Attachments (1)

trac_15335.patch (5.4 KB) - added by chapoton 6 years ago.

Download all attachments as: .zip

Change History (21)

Changed 6 years ago by chapoton

comment:1 Changed 6 years ago by chapoton

not ready, work in progress

comment:2 Changed 6 years ago by chapoton

  • Keywords startup added

comment:3 Changed 6 years ago by chapoton

  • Description modified (diff)

comment:4 Changed 6 years ago by chapoton

I suspect that the line

# For doctesting.
import sagenb

in sagenb/notebook/all.py could be the culprit. Could one juste remove this line ?

comment:5 Changed 6 years ago by jdemeyer

  • Branch set to u/jdemeyer/ticket/15335
  • Created changed from 10/27/13 21:07:04 to 10/27/13 21:07:04
  • Modified changed from 10/28/13 20:21:22 to 10/28/13 20:21:22

comment:6 Changed 6 years ago by jdemeyer

  • Authors set to Jeroen Demeyer
  • Commit set to 4b2c4703c52fe1f89586907f38b20c3ae11f32a2
  • Component changed from interact to misc
  • Description modified (diff)
  • Status changed from new to needs_review
  • Summary changed from do not import sagenb at startup to Do not import sagenb at startup

New commits:

4b2c470Do not import sagenb at Sage startup

comment:7 Changed 6 years ago by tscrim

  • Branch changed from u/jdemeyer/ticket/15335 to u/tscrim/ticket/15335
  • Commit changed from 4b2c4703c52fe1f89586907f38b20c3ae11f32a2 to b28a85ec7052309a9a2e70f62d41c8ce88e6a7c5
  • Reviewers set to Travis Scrimshaw

I get close to a 25% speedup in startup time. I've fixed the failing doctest from the patchbot in misc/dev_tools.py that I could reproduce, so if that also works for you Jeroen, then this is a positive review from me.


New commits:

b28a85eFixed doctest.

comment:8 Changed 6 years ago by chapoton

When running sage -t -a, I got a failure (same as the bot) in src/sage/all.py

comment:9 Changed 6 years ago by jdemeyer

  • Status changed from needs_review to needs_work

The problem is some of these doctests now depend on the --sagenb option given to the doctester.

comment:10 Changed 6 years ago by jdemeyer

This works:

./sage -t src/sage/all.py

This doesn't:

./sage -t --sagenb src/sage/all.py

comment:11 Changed 6 years ago by jdemeyer

  • Branch changed from u/tscrim/ticket/15335 to u/jdemeyer/ticket/15335
  • Modified changed from 01/08/14 19:47:38 to 01/08/14 19:47:38

comment:12 Changed 6 years ago by jdemeyer

  • Commit changed from b28a85ec7052309a9a2e70f62d41c8ce88e6a7c5 to da0d13387a2a52ec882b6c1d3ccd8c4651f17fdb

New commits:

214bf49Only add sagenb globals if actually testing a sagenb file
da0d133Merge branch 'u/tscrim/ticket/15335' of git://trac.sagemath.org/sage into ticket/15335

comment:13 Changed 6 years ago by jdemeyer

  • Status changed from needs_work to needs_review

Travis: agreed with your change.

http://git.sagemath.org/sage.git/commit/?id=214bf49a7f28dd69489a874aba8037b4f9b64194 needs review to fix the --sagenb option issue.

comment:14 Changed 6 years ago by tscrim

  • Status changed from needs_review to positive_review

LGTMTM.

comment:15 Changed 6 years ago by jdemeyer

  • Status changed from positive_review to needs_review

Never mind.

Last edited 6 years ago by jdemeyer (previous) (diff)

comment:16 Changed 6 years ago by jdemeyer

  • Status changed from needs_review to positive_review

comment:17 Changed 6 years ago by vbraun

  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:18 Changed 6 years ago by ppurka

This has broken latex in the notebook. See https://github.com/sagemath/sagenb/issues/195 for details.

comment:19 Changed 6 years ago by jdemeyer

Sorry guys :-(

comment:20 Changed 6 years ago by tscrim

Sorry!

Note: See TracTickets for help on using tickets.