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

Cytoscape bugfix for SIF files #3896

Merged
merged 2 commits into from
Apr 6, 2017
Merged

Conversation

anuprulez
Copy link
Member

This PR includes a bug fix for SIF to JSON file conversion for Cytoscape charts plugin.

  • Nodes are now correctly arranged according to SIF file specification

For the following SIF description:

nodeA xx nodeB
nodeB xx nodeC
nodeC xx nodeD
nodeD xx nodeA nodeB nodeC
nodeB xx nodeD
nodeZ

the resulting graph is:

271555ea-1ae9-11e7-801e-21c12f1b3c21

@dannon
Copy link
Member

dannon commented Apr 6, 2017

@anuprulez You should not need to do those dev merges like that, for what it's worth. This looks much better, though.

@dannon
Copy link
Member

dannon commented Apr 6, 2017

+1, works well now. Thanks for fixing it up @anuprulez.

@dannon dannon merged commit 483f25c into galaxyproject:dev Apr 6, 2017
@anuprulez
Copy link
Member Author

@dannon I pushed my changes and then did the merge with dev. Should I do the other way?

@dannon
Copy link
Member

dannon commented Apr 6, 2017

@anuprulez Unless there's actually a conflict, there is no need to do either -- the only merge in many situations will be the one that happens when a committer merges this back into dev (like what happens when "merge pull request" is clicked).

Even if there are conflicts, as long as they're just build artifacts, those are also trivial to resolve by the committer at merge time.

@anuprulez anuprulez deleted the bugfix_cytoscape branch April 7, 2017 09:34
@martenson martenson modified the milestone: 17.05 May 16, 2017
@martenson
Copy link
Member

Please watch milestone and label presence when merging. Or give @galaxybot a chance to catch that (he ignores closed issues).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants