Opened 6 years ago

Closed 5 years ago

Last modified 5 years ago

#14953 closed enhancement (fixed)

Graph drawing in javascript using d3.js

Reported by: ncohen Owned by:
Priority: major Milestone: sage-6.3
Component: graph theory Keywords:
Cc: bonfroy Merged in:
Authors: Nathann Cohen, Brice Onfroy Reviewers: Dima Pasechnik, Volker Braun
Report Upstream: N/A Work issues:
Branch: 76acf85 (Commits) Commit:
Dependencies: Stopgaps:

Description

As the title says.

This drawing method will be as good as the examples that are used to tune its parameters :-)

Nathann

Attachments (2)

trac_14953.patch (19.7 KB) - added by ncohen 5 years ago.
trac_14953_review1.patch (8.7 KB) - added by chapoton 5 years ago.

Download all attachments as: .zip

Change History (37)

comment:1 Changed 6 years ago by ncohen

  • Status changed from new to needs_review

comment:2 Changed 5 years ago by ncohen

  • Cc bonfroy added

Changed 5 years ago by ncohen

Changed 5 years ago by chapoton

comment:3 Changed 5 years ago by chapoton

very nice tool. Here is a purely cosmetic review patch

I still have to finish the review, but later

comment:4 Changed 5 years ago by ncohen

Well if you have some time to review that ?.... :-P

comment:5 Changed 5 years ago by ncohen

  • Branch set to u/ncohen/14953

Now a GIT patch.

comment:6 Changed 5 years ago by git

  • Commit set to 84b44ab8a2de5f53d203a34d405c4740ce137538

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

[changeset:84b44ab]Git directory layout
[changeset:465f9db]trac #14953 cleanup, pep8 and details
[changeset:89bbece]Graph drawing in javascript using d3.js

comment:7 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:8 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:9 Changed 5 years ago by rws

  • Status changed from needs_review to needs_work
  • Work issues set to rebase

comment:10 Changed 5 years ago by git

  • Commit changed from 84b44ab8a2de5f53d203a34d405c4740ce137538 to 44265a5175644283faea92a98cdc869b274ff984

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

44265a5trac #14953: Merged with 6.3.beta0

comment:11 Changed 5 years ago by ncohen

  • Status changed from needs_work to needs_review

Fixed. If you feel like giving it a review ... It is 10 months old already :-P

Nathann

comment:12 Changed 5 years ago by ncohen

  • Work issues rebase deleted

comment:13 Changed 5 years ago by dimpase

One must be online to use this stuff, right?

comment:14 Changed 5 years ago by ncohen

Right now yes, but there is no "technical" reason. Right now, it loads the javascript files d3.js from some URL, all this is not contained in the patch. We can later change that or write a spkg containing those files, and there will be no need of that anymore.

Nathann

comment:15 Changed 5 years ago by dimpase

looks good. I'll check that the docs build, and then it should be good to go.

comment:16 Changed 5 years ago by dimpase

  • Branch changed from u/ncohen/14953 to u/dimpase/14953

comment:17 Changed 5 years ago by dimpase

  • Status changed from needs_review to positive_review

comment:18 Changed 5 years ago by dimpase

  • Reviewers set to Dima Pasechnik

comment:19 Changed 5 years ago by ncohen

Wow ! Thank you very much ! :-)

If you do not feel like sleeping there is a combinatorial_map war on Sage-devel. It got so rewarding I even closed #16403 just stop talking.

Nathann

comment:20 Changed 5 years ago by vbraun

  • Status changed from positive_review to needs_work

Pops up a browser window when running "make ptestlong"... please find a better way of automated testing ;-)

comment:21 Changed 5 years ago by vbraun

Also, anything that requires internet must be marked as # optional - internet.

comment:22 Changed 5 years ago by ncohen

Hmmmmm... Is there a way to test anything that this patch does ?... :-/

Nathann

comment:23 Changed 5 years ago by vbraun

You can split the html generation from the show() function and doctest it as usual. Show can just check DOCTEST_MODE to avoid starting the browser.

Also, don't load the html template from SAGE_SRC. Thats the source code, if you need files at runtime then they must be installed under SAGE_LOCAL. You should probably put it somewhere under src/ext and then use SAGE_EXTCODE to refer to it.

comment:24 Changed 5 years ago by ncohen

  • Branch changed from u/dimpase/14953 to u/ncohen/14953
  • Commit changed from 44265a5175644283faea92a98cdc869b274ff984 to 76acf85eecadfaf3c64637b6f564ae51b6b5b940
  • Status changed from needs_work to needs_review

Here is a commit to implement all that. The down side is that you have to run "make" for the .html file to be copied in SAGE_EXTCODE ..... That's what Volker wanted.

Nathann


New commits:

9cbcc36trac #14953: Merged with 6.3.beta2
76acf85trac #14953: Moving the .html to EXT_CODE and using DOCTEST_MODE

comment:25 Changed 5 years ago by vbraun

  • Reviewers changed from Dima Pasechnik to Dima Pasechnik, Volker Braun
  • Status changed from needs_review to positive_review

lgtm

comment:26 Changed 5 years ago by vbraun

  • Branch changed from u/ncohen/14953 to 76acf85eecadfaf3c64637b6f564ae51b6b5b940
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:27 follow-up: Changed 5 years ago by tmonteil

  • Commit 76acf85eecadfaf3c64637b6f564ae51b6b5b940 deleted

Hi, i do not like the need to fetch a file from the internet to let this work. My question for a future ticket is the following: if i understand correctly, src/ext is a kind of place where we put external source code that is not big enough to deserve a package, am i right ? In this case, is this the right place to copy the file d3.v3.min.js for offline use ?

comment:28 in reply to: ↑ 27 Changed 5 years ago by ncohen

Yoooooooooooooo !

Hi, i do not like the need to fetch a file from the internet to let this work. My question for a future ticket is the following: if i understand correctly, src/ext is a kind of place where we put external source code that is not big enough to deserve a package, am i right ? In this case, is this the right place to copy the file d3.v3.min.js for offline use ?

HMmmmmmm.... I am interested by Volker's answer, but I would say that stuff in src/ should be reviewed... Sooooooo no third-party code should be there. Besides, it is very easy to write spkg now with git.

This being said, I do not know which files we are allowed to read at runtime...

Nathann

comment:29 Changed 5 years ago by vbraun

No, SAGE_EXT is where we install non-Python Sage source code. Its not a dump for third-party stuff that you don't want to package.

comment:30 Changed 5 years ago by ncohen

Volker : if we package this code how can we access it at runtime ? What would the spkg-install do, and where would it copy the .js file ?

Nathann

comment:31 Changed 5 years ago by tmonteil

OK, so we have to make a spkg that just copy the d3.v3.min.js file into ./local/share/d3.js/ directory, then we can use from sage.misc.misc import SAGE_SHARE to get it. It that the correct procedure ? Or do we need to recompile the .js file from sources ?

comment:32 Changed 5 years ago by dimpase

there could be copyright issues that would preclude one from distributing it with Sage, so it will need to stay an optional Sage package, whether you like it or not.

IANAL. Here is the licence: https://github.com/mbostock/d3/blob/master/LICENSE

comment:33 Changed 5 years ago by tmonteil

IANAL either, this is a Modified BSD License, which is GPL-compatible according to the FSF (see also wikipedia). So it is OK to have it as a standard spkg, right ?

comment:34 Changed 5 years ago by tmonteil

This is now #16434 so that i will learn how to package.

comment:35 Changed 5 years ago by tmonteil

See also #16438 which links both features.

Note: See TracTickets for help on using tickets.