-
Notifications
You must be signed in to change notification settings - Fork 1
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 index file for bam visualizations #32
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Optional small tweaks.
context/on_startup.py
Outdated
), | ||
"url": node_data["file_url"] | ||
} | ||
if '.bam' == os.path.splitext(track['name']): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if '.bam' == os.path.splitext(track['name']): | |
if '.bam' == os.path.splitext(node_data["file_url"]): |
Probably safer to go to the source for this: The track['name']
is really just a human readable string, and the format could change.
(If you do accept this, maybe define a variable for node_data["file_url"]
, since it's being used three times.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done!
context/on_startup.py
Outdated
# assume that there is only one auxiliary file for bam igv and it's | ||
# the .bai file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# assume that there is only one auxiliary file for bam igv and it's | |
# the .bai file | |
# Assume that there is only one auxiliary file for bam igv and it's the .bai file. |
I generally prefer to have a capital letter and period, when a comment is a sentence, but your call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done!
By fixing refinery-platform/refinery-platform#2718 and updating the API, all we need to do here is add the indexURL for bam files. We should probably hold off on merging this until after the next release (i.e after Monday/Tuesday). No reason we can't review it though!