Opened 5 years ago

Closed 5 years ago

#17171 closed enhancement (fixed)

Upgrade dot2tex to 2.9.0

Reported by: tscrim Owned by: sage-combinat
Priority: major Milestone: sage-6.8
Component: packages: optional Keywords: dot2tex
Cc: sage-combinat, nthiery, aschilling, bsalisbury1, jpflori Merged in:
Authors: Travis Scrimshaw Reviewers: Frédéric Chapoton, Jeroen Demeyer
Report Upstream: N/A Work issues:
Branch: f5a9e93 (Commits) Commit: f5a9e93c642095ed7757521f73c87b961d41582c
Dependencies: Stopgaps:

Description (last modified by tscrim)

dot2tex 2.9.0 was release in May, 2014. Currently we are at 2.9.0dev, and we should make sure we're at the latest stable release.

Upstream tarball: https://pypi.python.org/packages/source/d/dot2tex/dot2tex-2.9.0.tar.gz#md5=2dbaeac905424d0410751235bde4b8b2

Change History (35)

comment:1 Changed 5 years ago by tscrim

  • Authors set to Travis Scrimshaw
  • Branch set to public/optional_spkg/dot2tex-2.9.0-17171
  • Commit set to 005d70129c9478552fb5e4dde63ececbd117a967
  • Description modified (diff)
  • Status changed from new to needs_review

All of our current patches no longer apply because either they were merged upstream or were changes to the testing framework (which has completely changed from the dev version). However I added a new patch which removes the test_semicolon because this is the known, but not yet fixed, Issue #5.


New commits:

005d701Upgrade dot2tex to 2.9.0.

comment:2 Changed 5 years ago by jpflori

  • Cc jpflori added

comment:3 Changed 5 years ago by leif

  • Milestone changed from sage-6.4 to sage-6.6

comment:4 Changed 5 years ago by tscrim

ping

comment:5 Changed 5 years ago by chapoton

  • Reviewers set to Frédéric Chapoton
  • Status changed from needs_review to needs_work

with graphviz and this new dot2tex, some trivial doctest failures:

sage -t --optional=sage,dot2tex,graphviz ./graphs/dot2tex_utils.py ./graphs/graph_latex.py ./graphs/generic_graph.py ./categories/crystals.py ./combinat/perfect_matching.py ./combinat/crystals/crystals.py ./combinat/rigged_configurations/kleber_tree.py ./combinat/posets/linear_extensions.py ./combinat/posets/posets.py


sage -t graphs/generic_graph.py # 6 doctests failed sage -t combinat/posets/posets.py # 1 doctest failed


comment:6 Changed 5 years ago by chapoton

  • Milestone changed from sage-6.6 to sage-6.8

comment:7 Changed 5 years ago by git

  • Commit changed from 005d70129c9478552fb5e4dde63ececbd117a967 to 08646109a599914ccb43ee90c94596402585f43e

Branch pushed to git repo; I updated commit sha1. New commits:

855e55dMerge branch 'public/optional_spkg/dot2tex-2.9.0-17171' of trac.sagemath.org:sage into public/optional_spkg/dot2tex-2.9.0-17171
0864610Fixed trivially failing doctests.

comment:8 Changed 5 years ago by tscrim

I fixed the trivially failing doctests (I also made the view() example not tested because it opens a separate window). Looks like this will need to rebased on beta3, which I will do tonight.

comment:9 Changed 5 years ago by chapoton

Are you sure that <BLANKLINE> is needed ?

comment:10 Changed 5 years ago by git

  • Commit changed from 08646109a599914ccb43ee90c94596402585f43e to 1938b1eecc5e4ee4a65df51885a9a8efae7a1d71

Branch pushed to git repo; I updated commit sha1. New commits:

1938b1eMerge branch 'public/optional_spkg/dot2tex-2.9.0-17171' into 6.8.b3

comment:11 Changed 5 years ago by git

  • Commit changed from 1938b1eecc5e4ee4a65df51885a9a8efae7a1d71 to 8eef83dddd16841649015d7dfea1a32c62fd3e84

Branch pushed to git repo; I updated commit sha1. New commits:

159fa6aMerge branch 'develop' into public/optional_spkg/dot2tex-2.9.0-17171
758cf08Merge branch 'public/optional_spkg/dot2tex-2.9.0-17171' of trac.sagemath.org:sage into public/optional_spkg/dot2tex-2.9.0-17171
8eef83dRemoved blanklines.

comment:12 Changed 5 years ago by tscrim

  • Status changed from needs_work to needs_review

I had done my merge, checking, and was about to push when you did your merge :P.

Turns out the <BLANKLINE>'s are not needed.

comment:13 Changed 5 years ago by chapoton

I have a failure:

sage -t combinat/posets/posets.py
**********************************************************************
File "combinat/posets/posets.py", line 1259, in sage.combinat.posets.posets.FinitePoset._latex_
Failed example:
    print P._latex_() #optional - dot2tex graphviz
Expected:
    \begin{tikzpicture}[>=latex,line join=bevel,]
    %%
    \node (node_1) at (6.0...bp,57.0...bp) [draw,draw=none] {$2$};
      \node (node_0) at (6.0...bp,7.0...bp) [draw,draw=none] {$1$};
      \draw [black,->] (node_0) ..controls (6.0...bp,20.2...bp) and (6.0...bp,30.9...bp)  .. (node_1);
    %
    \end{tikzpicture}
Got:
    <BLANKLINE>
    \begin{tikzpicture}[>=latex,line join=bevel,]
    %%
    \node (node_1) at (6.0bp,57.0bp) [draw,draw=none] {$2$};
      \node (node_0) at (6.0bp,7.0bp) [draw,draw=none] {$1$};
      \draw [black,->] (node_0) ..controls (6.0bp,19.947bp) and (6.0bp,30.897bp)  .. (node_1);
    %
    \end{tikzpicture}
    <BLANKLINE>

because of the 19 instead of 20

And moreover, it pops me out 10 pdf pictures in new windows..

I did

sage -bt --optional=sage,dot2tex,graphviz ./graphs/__init__.py ./graphs/dot2tex_utils.py ./graphs/graph.py ./graphs/generic_graph.py ./graphs/digraph.py ./graphs/graph_latex.py ./categories/crystals.py ./combinat/perfect_matching.py ./combinat/crystals/crystals.py ./combinat/partition.py ./combinat/posets/posets.py ./combinat/posets/linear_extensions.py ./combinat/rigged_configurations/kleber_tree.py 

comment:14 Changed 5 years ago by git

  • Commit changed from 8eef83dddd16841649015d7dfea1a32c62fd3e84 to b7632caa4fe14de27a22d61074bc22e88ff14f3a

Branch pushed to git repo; I updated commit sha1. New commits:

b7632caMaking the control points more forgiving.

comment:15 Changed 5 years ago by tscrim

That should make that doctest pass.

For better or worse, that was the current behavior before this ticket. However, do you want me to go through and make similar changes with opening external windows?

comment:16 Changed 5 years ago by jdemeyer

Small detail:

# not tested - dot2tex, opens external window

should probably be

# optional - dot2tex, not tested (opens external window)

or something.

comment:17 follow-up: Changed 5 years ago by chapoton

well, maybe it would be good, if optionals tests are now going to be run more often..

comment:18 in reply to: ↑ 17 Changed 5 years ago by jdemeyer

Replying to chapoton:

well, maybe it would be good, if optionals tests are now going to be run more often..

graphviz isn't optional (it's experimental), so graphviz will not be tested by default.

comment:19 Changed 5 years ago by git

  • Commit changed from b7632caa4fe14de27a22d61074bc22e88ff14f3a to 7cee4abaf00aa344a6db7bdff0ff4f15e8d018f8

Branch pushed to git repo; I updated commit sha1. New commits:

7cee4abChanging optional tests to not tested because they open an external window.

comment:20 Changed 5 years ago by tscrim

I've marked all applicable optional tests as per Jeroen's suggestion.

Last edited 5 years ago by tscrim (previous) (diff)

comment:21 Changed 5 years ago by jdemeyer

  • Status changed from needs_review to needs_work

Replace

optional - dot2tex, graphviz

by

optional - dot2tex graphviz

(like it was before)

comment:22 follow-up: Changed 5 years ago by tscrim

I'm assuming you're refering to

# optional - dot2tex, graphviz, not tested (opens external window)

Do you want it like

# optional - dot2tex graphviz, not tested (opens external window)

(which looks somewhat strange) or

# optional - dot2tex graphviz not tested (opens external window)

(which looks even more strange) or what? I didn't add commas anywhere else that I could see...

comment:23 in reply to: ↑ 22 Changed 5 years ago by jdemeyer

Replying to tscrim:

Do you want it like

# optional - dot2tex graphviz, not tested (opens external window)

Yes

comment:24 Changed 5 years ago by git

  • Commit changed from 7cee4abaf00aa344a6db7bdff0ff4f15e8d018f8 to 96cb142f440fb0423e829485bcfe526746430c33

Branch pushed to git repo; I updated commit sha1. New commits:

96cb142Removing commas.

comment:25 Changed 5 years ago by tscrim

  • Status changed from needs_work to needs_review

Done.

comment:26 Changed 5 years ago by jdemeyer

Doctests are good now.

Frédéric, can you do the final review?

comment:27 Changed 5 years ago by chapoton

  • Status changed from needs_review to positive_review

ok, looks good, as far as I can tell.

comment:28 Changed 5 years ago by jdemeyer

  • Reviewers changed from Frédéric Chapoton to Frédéric Chapoton, Jeroen Demeyer

comment:29 Changed 5 years ago by vbraun

  • Status changed from positive_review to needs_work

src/sage/doctest/sources.py fails

comment:30 Changed 5 years ago by git

  • Commit changed from 96cb142f440fb0423e829485bcfe526746430c33 to 2eb080c0d39e7aa0f2840ffca5605abfc54e3c76

Branch pushed to git repo; I updated commit sha1. New commits:

2eb080cFix doctest/sources failure.

comment:31 Changed 5 years ago by tscrim

  • Status changed from needs_work to positive_review

Fixed. Trivial failure (and there are similar results that are there).

comment:32 Changed 5 years ago by jdemeyer

  • Status changed from positive_review to needs_work

I missed this the last time, but there should be a comma (or other separator) at the @@@ below:

view(H, tight_page=True) # optional - dot2tex@@@ not tested (opens external window)

comment:33 Changed 5 years ago by git

  • Commit changed from 2eb080c0d39e7aa0f2840ffca5605abfc54e3c76 to f5a9e93c642095ed7757521f73c87b961d41582c

Branch pushed to git repo; I updated commit sha1. New commits:

f5a9e93Adding a missing comma and reverting previous change.

comment:34 Changed 5 years ago by tscrim

  • Status changed from needs_work to positive_review

Fixed.

comment:35 Changed 5 years ago by vbraun

  • Branch changed from public/optional_spkg/dot2tex-2.9.0-17171 to f5a9e93c642095ed7757521f73c87b961d41582c
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.