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

Add genesight #278

Closed
wants to merge 20 commits into from
Closed

Add genesight #278

wants to merge 20 commits into from

Conversation

jdr0887
Copy link

@jdr0887 jdr0887 commented Jan 22, 2025

adding LINCS & MetabolomicsWorkbench implementation where the code is nearly identical per parser. A cfde config file specifies the details.

Copy link
Contributor

@EvanDietzMorris EvanDietzMorris left a comment

Choose a reason for hiding this comment

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

This all looks great. A couple questions..

What is the change to the kgx_file_converter for? I'm not sure I understand that.

I like the change to the automat graph ids, but I'm hesitant because it will break the continuity of the existing graph output directories and make them look like different graphs. Is there a technical reason you think it should be done or just style? I think I agree either way but I'm curious.

@EvanDietzMorris
Copy link
Contributor

Note that I just pushed some changes to that specific part of the kgx_file_converter (and immediately saw a bug that I'm about to push a fix for hah) but the changes shouldn't affect what you have here

@jdr0887
Copy link
Author

jdr0887 commented Jan 23, 2025

What is the change to the kgx_file_converter for? I'm not sure I understand that.

Me either. I am afraid I may have branched off of the wrong branch.

I like the change to the automat graph ids, but I'm hesitant because it will break the continuity of the existing graph output directories and make them look like different graphs. Is there a technical reason you think it should be done or just style? I think I agree either way but I'm curious.

I am going to move these changes to a new PR that is from a clean master branch.

@jdr0887 jdr0887 closed this Jan 23, 2025
@jdr0887 jdr0887 deleted the add-genesight branch January 23, 2025 00:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants