Skip to content

Add specific datatypes for Cytoscape and Kepler.gl #20117

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

Merged
merged 13 commits into from
May 5, 2025

Conversation

guerler
Copy link
Contributor

@guerler guerler commented Apr 30, 2025

This PR adds two new datatype definitions: cytoscapejson and keplercsv, which are subtypes of json and csv, respectively. These formats are consumed by Cytoscape and Kepler.gl. Introducing specific datatypes improves user guidance and prevents the list of selectable datatypes from being cluttered with incompatible formats.

While only tangentially related to this PR, I believe there is a broader need for a systematic, large-scale evaluation of all datatypes and sniffers in an all-against-all fashion. Such an analysis would improve the robustness and accuracy of datatype detection across the platform.

How to test the changes?

(Select all options that apply)

  • I've included appropriate automated tests.
  • This is a refactoring of components with existing test coverage.
  • Instructions for manual testing are as follows:
    1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

  • I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

@guerler guerler added this to the 25.0 milestone Apr 30, 2025
@guerler guerler requested a review from bgruening April 30, 2025 17:11
@guerler guerler marked this pull request as ready for review April 30, 2025 17:40
class GeoCSV(CSV):
"""
CSV format compatible with Kepler.gl, expected to contain latitude and longitude fields.
https://docs.kepler.gl/docs/keplergl-schema
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This link does not work for me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I replaced the link.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a real spec somewhere? I'm not sure we need a sniffer here. I guess at least for the start we can just add a subclass of csv that is called geocsv?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the link, but if you prefer I can remove the sniffer and just introduce it as sub class without sniffer instead.

class CytoscapeJson(Json):
"""
Cytoscape JSON format for network visualization, typically containing 'nodes' and 'edges' in a JSON object.
https://js.cytoscape.org/#notation/elements-json
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can add this link also to the datatypes_conf file as description and description_url

@guerler guerler force-pushed the visualization_datatypes branch from e54fce5 to 5637849 Compare May 2, 2025 16:09
@guerler guerler force-pushed the visualization_datatypes branch from 5637849 to b94475c Compare May 3, 2025 18:46
@guerler guerler merged commit dd11f85 into galaxyproject:dev May 5, 2025
52 of 55 checks passed
@guerler guerler deleted the visualization_datatypes branch May 5, 2025 13:59
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.

2 participants