Add svg image generation #88

IvoLeist · 2020-05-22T16:50:57Z

About

This PR is an extension of this PR which provides access to the existing Cytoscape.js image generation APIs, cy.png() and cy.jpg(), via Dash callbacks. It allows the current view of a Cytoscape graph to be saved as an image, stored as a base64 string or data URL, and/or downloaded as a file.
On top it enables the generation of an .svg file using that cytoscape plugin

Description of changes

This PR adds to the generateImage and imageData properties the possibility to generate an svg image.

Pre-Merge checklist

The project was correctly built with yarn run build:all. !npm is buggy
If there was any conflict, it was solved correctly
All changes were documented in CHANGELOG.md.
All tests on CircleCI have passed.
All Percy visual changes have been approved.
Two people have 💃'd the pull request. You can be one of these people if you are a Dash Cytoscape core contributor.

Reference Issues

No current issues related to this PR.

Other comments

Please note that it is only building correctly if yarn is used.
If you use npm and need compound nodes to be fixed at one place
building dash-cytoscape with npm will result in unexpected behaviours
( = freely floating children nodes)

This reverts commit 4fa1fed.

Image download

Add python files

…est imageDownload

…kage.json

IvoLeist · 2020-05-22T17:37:34Z

@alexcjohnson can you resolve these two conflicts?
Or what do I have to do to resolve them?

Conflicting files
dash_cytoscape/package.json
yarn.lock

xhluca · 2020-05-25T21:52:28Z

Hi Ivo! Could you rebase your branch on the current masters? if that doesn't resolve the conflicts, I can look into it.

xhluca · 2020-05-25T21:53:09Z

That option should be available on Github; if it's not, feel free to use git rebase

xhluca · 2020-05-25T21:55:50Z

Also once the conflicts are resolved, could you double check if the elements in my review from the previous PR were incorporated? Thanks!

…ased on usage-event.py

IvoLeist · 2020-05-26T11:50:42Z

@xhlulu These are the ones you were mentioning, right?
If so I just added them.

"In demos, it would be nice to have a usage-image-export.py example that shows how to build a simple dash app and use the image functionality in various ways."
"Once you made that app, could you add it test_usage to make sure the app renders correctly? It should simply be 2 lines of code, since all it does is rendering (it doesn't actually test any callback)."

IvoLeist · 2020-05-26T11:55:51Z

Just for the records a small disclaimer:

I have not managed to modify the react code in such a way that I was able to generate the string of the svg. Therefore, I added in usage-image-export and in Cytoscape.react.js the disclaimer that for the option 'store' only jpg and png are supported.

Maybe someone else has an idea or want to add this in a follow up release.

xhluca · 2020-05-27T00:30:03Z

Thanks for adding to those suggestions! Would it be possible to share a GIF of the usage app you created? You could even add it at the gallery at the end of README!

Also, just to make sure: Have you ran npm run build:all before committing the last changes?

xhluca

Hi Ivo! Thank you for not only continuing this PR, but also adding substantial changes to support exporting in SVG. This is super appreciated.

I added a few comments specifically about the changes from the SVG module; whenever you have the time, could you take a look at them? Overall, I think we are close to merging the PR, as long as the comments are addressed and the tests all pass.

I saw that you added package-lock.json; are you perhaps using npm? At the start of the project we settled on using yarn, so it would be great if we don't have multiple lock files. Let me know if you have any question about yarn.

setup.py

xhluca · 2020-05-27T00:43:25Z

src/lib/components/Cytoscape.react.js

 * React.js requirements: react-cytoscapejs
 */
+import cytoscape from 'cytoscape';
+import svg from 'cytoscape-svg';


I'm wondering if it's a good idea to add svg to the main module, since we want to minimize the file size when loading dash-cytoscape; also, adding one more package here means more dependencies to keep track. This is the reason why we have an extra_index.js that loads all of the external dependencies.

Do you believe this should stay in the main file, or would it be better to have it in the extra_index.js?

That is a good idea, I was not aware of the exta_index.js before.
I just tried to add it to extra index.js and it works as expected :)

However, following that logic I guess it would be a good idea to put the plugins in an new file. On top it kinda feels wrong to enable the svg export feature with cyto.load_extra_layouts().

Since I do not 100% understand how to implement this I just added a plugins_index.js file as a reminder.

Ivo, please add a function directly to __init__ similar to this one, except here you would use _plugins instead of _extra. The function name could be load_plugins, and an example on how to use it could be added to the readme right below this section.

Feel free to also load it in your new usage app, if you have time. Thanks!

You are totally right on the confusion around extra_index.js. I think it's something I can add in a future release of Cytoscape, inside CONTRIBUTION.md, so people that want to add more plugin could easily do it.

Actually, I think this might not be the best way. Maybe it's better to keep it inside extra_index.js. More discussions below.

IvoLeist · 2020-05-27T14:34:28Z

Hi Ivo! Thank you for not only continuing this PR, but also adding substantial changes to support exporting in SVG. This is super appreciated.

Thank you @xhlulu for the encouraging words 😃
but the heavy lifting was done by @clegaspi

These should be now all required changes meaning I am ready to merge 🚀
So 🤞 that all the test are completing successfully.

Update:
Pylint is falling because dash forces me to add add get_jpg_clicks, get_png_clicks, get_svg_clicksto def get_image though I do not need them in the function:

Your code has been rated at 10.00/10

************* Module demos.usage-image-export
demos/usage-image-export.py:169:19: W0613: Unused argument 'get_jpg_clicks' (unused-argument)
demos/usage-image-export.py:169:35: W0613: Unused argument 'get_png_clicks' (unused-argument)
demos/usage-image-export.py:169:51: W0613: Unused argument 'get_svg_clicks' (unused-argument)

Your code has been rated at 9.97/10
Exited with code exit status 4

Should I just add a random print statement to prevent that pylint errors
or do you have a better suggestion?

IvoLeist · 2020-05-27T14:52:40Z

Regarding:

Also, just to make sure: Have you ran npm run build:all before committing the last changes?

You meant yarn run build:all, right? If so, yes I have done that

Regarding:

I saw that you added package-lock.json; are you perhaps using npm? At the start of the project we settled on using yarn, so it would be great if we don't have multiple lock files. Let me know if you have any question about yarn.

I removed it

xhluca · 2020-05-27T18:13:06Z

Thanks! yarn run build:all is good, but I don't think it's the reason why the package-lock.json was created. I haven't used npm in a while, so i'm not totally sure when it gets created.

Adding random prints is fine.

IvoLeist · 2020-05-27T18:37:37Z

@xhlulu just added the random print statement 🤞

I haven't used npm in a while, so i'm not totally sure when it gets created.

When I installed it the first time I used npm and was tearing my hairs out
why suddenly the compounds nodes were messed up...
before I finally realized that I have to use yarn 🤦‍♂️

xhluca · 2020-05-27T18:57:48Z

CC: @alexcjohnson @Marc-Andre-Rivet

So in this PR, we need to load an external package, which is usually done by calling CytoscapeJS.use(external_package). Currently, it's done in the following way to load external layouts:

// Imports
...

// Load
CytoscapeJS.use(coseBilkent);
CytoscapeJS.use(cola);
CytoscapeJS.use(dagre);
CytoscapeJS.use(euler);
CytoscapeJS.use(klay);
CytoscapeJS.use(spread);

export {
    Cytoscape
};

The full extra_index.js file is here.

Then, the file is loaded with a load_extra_layout function in the __init__.py file.

However, two options are possible:

Add CytoscapeJS.use(svg) in extra_index.js, and the JS file is loaded when you call load_extra_layout.
Create a new file plugin_index.js where only svg is loaded.

In the case of the second option, we'd need to essentially:

create plugin_index.js
Create a new webpack.dev.plugin.config.js and webpack.prod.plugin.config.js files to build plugin_index.js into dash_cytoscape_plugin.[dev|min].js.
Modify package.json so that it calls the new webpack files.
Add dash_cytoscape_plugin into MANUSCRIPT.in.
create a new function load_plugins in __init__.py, and add that to the docs/readme.

For simplicity, I think it's better to go with the first option, and in a future release we could deprecate the function load_extra_layout, and opt for something more generic like load_extra or load_plugins, which would essentially load all external files at the same time.

IvoLeist · 2020-05-27T19:01:46Z

@xhlulu there seems to be some flakyness with ci 😬
Can you please restart it?

selenium.common.exceptions.WebDriverException: Message: unknown error: Chrome failed to start: crashed.
  (unknown error: DevToolsActivePort file doesn't exist)
  (The process started from chrome location /usr/bin/google-chrome is no longer running, so ChromeDriver is assuming that Chrome has crashed.)

xhluca · 2020-05-27T19:43:43Z

@IvoLeist After discussing about this, I think we will move forward with option 1, so you won't need to make any change concerning index.js (i.e. no need to mess with the webpacks 😆 ).

alexcjohnson · 2020-05-27T19:44:34Z

Add CytoscapeJS.use(svg) in extra_index.js, and the JS file is loaded when you call load_extra_layout.

Yes, I think this is fine. As discussed with @xhlulu on slack, long-term we can expect to turn each of the extra layouts and the SVG generation code into async chunks that would get loaded on demand, in which case we can get rid of load_extra_layout entirely. So in the interim this is a fine place to put the SVG functionality.

xhluca · 2020-05-27T19:52:36Z

This will be a 💃 for me. @IvoLeist Let me know if there's anything else you'd like to add, but otherwise I can merge this.

IvoLeist · 2020-05-27T19:53:47Z

@xhlulu Fine for me let's go 🚀

clegaspi · 2020-05-28T02:09:56Z

You all are awesome! Thanks for working on this!

clegaspi and others added 16 commits July 18, 2019 17:35

Add image download and callback functionality

05a8083

Minor updates

1541f1e

Make api more concise

4533217

Update changelog

a7517a5

Add JSON

4fa1fed

Revert "Add JSON"

e710e53

This reverts commit 4fa1fed.

Merge pull request clegaspi#3 from clegaspi/image_download

b926824

Image download

Compile js and python files

9a88fd1

Compile js and python files (add files this time)

06afdd9

Merge pull request clegaspi#4 from clegaspi/image_download

f97aba9

Add python files

added example app to test image generation

6c57ccb

modified setup.py to include utils in modified package

aaed204

added cytoscape-svg as extension

ca5b267

extended cytotscape react to enable svg export,added example app to t…

6de00f9

…est imageDownload

renamed buttons in example app

76ff86a

rebuild it using yarn fixed compound nodes issue

fb48d5a

IvoLeist mentioned this pull request May 22, 2020

Access cytoscape.js png/jpg image generation API from dash #65

Closed

6 tasks

updated changelog, moved examples into demos/, updated E-mails in pac…

dbab9a3

…kage.json

xhluca self-requested a review May 25, 2020 21:53

IvoLeist added 3 commits May 26, 2020 10:44

fetched and merged with upstream/master

ee0397f

updated comments in Cytoscape.react.js, added usage-image-export.py b…

aa5d5b9

…ased on usage-event.py

added usage-image-export.py to test_usage

e55f159

xhluca suggested changes May 27, 2020

View reviewed changes

IvoLeist added 3 commits May 27, 2020 12:16

moved cytoscape-svg into extra_index.js

102bf70

resolved code linting issues

ecb7f0e

added usage-image-export gif to README

bf343f2

added random print statement in usage-image-export.py to pass pylint

7267497

xhluca approved these changes May 27, 2020

View reviewed changes

xhluca merged commit 593330b into plotly:master May 28, 2020

xhluca mentioned this pull request Feb 3, 2021

"fcose" layout not in the list of possible layouts. #119

Closed

Uh oh!

Add svg image generation #88

Add svg image generation #88

Uh oh!

Conversation

IvoLeist commented May 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

About

Description of changes

Pre-Merge checklist

Reference Issues

Other comments

Uh oh!

IvoLeist commented May 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xhluca commented May 25, 2020

Uh oh!

xhluca commented May 25, 2020

Uh oh!

xhluca commented May 25, 2020

Uh oh!

IvoLeist commented May 26, 2020

Uh oh!

IvoLeist commented May 26, 2020

Uh oh!

xhluca commented May 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xhluca left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

xhluca May 27, 2020

Choose a reason for hiding this comment

Uh oh!

IvoLeist May 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xhluca May 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xhluca May 27, 2020

Choose a reason for hiding this comment

Uh oh!

xhluca May 27, 2020

Choose a reason for hiding this comment

Uh oh!

IvoLeist commented May 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

IvoLeist commented May 27, 2020

Uh oh!

xhluca commented May 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

IvoLeist commented May 27, 2020

Uh oh!

xhluca commented May 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

IvoLeist commented May 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xhluca commented May 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexcjohnson commented May 27, 2020

Uh oh!

xhluca commented May 27, 2020

Uh oh!

IvoLeist commented May 27, 2020

Uh oh!

clegaspi commented May 28, 2020

Uh oh!

Uh oh!

IvoLeist commented May 22, 2020 •

edited

Loading

IvoLeist commented May 22, 2020 •

edited

Loading

xhluca commented May 27, 2020 •

edited

Loading

IvoLeist May 27, 2020 •

edited

Loading

xhluca May 27, 2020 •

edited

Loading

IvoLeist commented May 27, 2020 •

edited

Loading

xhluca commented May 27, 2020 •

edited

Loading

xhluca commented May 27, 2020 •

edited

Loading

IvoLeist commented May 27, 2020 •

edited

Loading

xhluca commented May 27, 2020 •

edited

Loading