Adding Header Lines #142
Adding Header Lines #142
Conversation
|
For some reason I am not able to remove the header line of each files from the cytoscape files. |
|
I still cannot get the header line to be skipped, it might be something on my end version wise, but I'm not sure |
|
rebuild and retag the image for docker |
|
I resolved a merge conflict that occurred after updating the py4cytoscape image tag. Were you able to get Cytoscape working now? |
|
Let me check after the merge, but before the merge I was able to resolve it.
Get Outlook for iOS<https://aka.ms/o0ukef>
…________________________________
From: Anthony Gitter ***@***.***>
Sent: Friday, January 19, 2024 1:49:06 PM
To: Reed-CompBio/spras ***@***.***>
Cc: Neha Talluri ***@***.***>; Assign ***@***.***>
Subject: Re: [Reed-CompBio/spras] Adding Header Lines (PR #142)
I resolved a merge conflict that occurred after updating the py4cytoscape image tag.
Were you able to get Cytoscape working now?
—
Reply to this email directly, view it on GitHub<#142 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ASZQFXFKO72XSCJNIFN6LRTYPLS5FAVCNFSM6AAAAABBAHKEFSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMBRGE3DGMJZGU>.
You are receiving this because you were assigned.Message ID: ***@***.***>
|
spras/domino.py
Outdated
| edges_df.columns = ['Node1', 'Node2', 'Rank', 'Direction'] | ||
| edges_df.to_csv(standardized_pathway_file, sep='\t', header=True, index=False) | ||
| else: | ||
| edges_df.to_csv(standardized_pathway_file, sep='\t', header=None, index=False) |
There was a problem hiding this comment.
Will this output a completely blank file? Should we output the header row now instead? We could do that without pandas if it is hard to do with pandas.
There was a problem hiding this comment.
Randomly the output file for domino (and for only domino) will add a '\n' to the file when header = True. This was our version of a quick fix to allow for that to not happen.
|
I've finished the review/comments of the pull request. When you review it again, could you pay special attention to the domino section? I'm not sure that my changes are that good but I was hesitant to make extensive changes to the code. |
|
Ready for Code review |
|
I need to update the contributing guide |
|
We need to revisit #143 (comment) before merging this because I expect to merge #143 first |
|
We should add documentation of the standard pathway output format now that it includes rank, direction, and universal header rows. Where would that go? In the It can copy the input file example from |
|
the |
|
@ntalluri I re-reviewed all of the files and pushed a few commits with code cleanup. Can you please review my changes to see if they look okay? If the GitHub actions tests pass, then I will run the workflow locally to inspect if the output pathways with headers look good. It is hard to be fully confident all of the parse output logic will work as intended with empty and non-empty pathways for all algorithms. |
|
The changes look good. It seems like the parse output logic is working how I thought it would, but I'm sure what you mean on it not being fully confident on it working. |
|
I meant that even though the parse output code changes look good, I don't fully remember all of the behaviors of the various pathway reconstruction algorithms. There could be some corner cases I'm forgetting that would affect how they write headers or deal with empty pathways. It's hard to consider all the possibilities in a big change like this. |
|
One case to consider is if the raw pathway file only containing a header line and nothing else. It seems like the code already deals with this for omics integrator 2, but I haven't written any test cases for it. |
|
That's a good example of algorithm-specific behavior. Some algorithms may output an empty file. Others may output a header only. Their output parsers prior to this change should have already handled that though. |
No description provided.