supported to run ROSE2 in python3 environment#67
supported to run ROSE2 in python3 environment#67gauravj49 wants to merge 3 commits intoBradnerLab:masterfrom
Conversation
jdimatteo
left a comment
jdimatteo
left a comment
There was a problem hiding this comment.
Overall changes look good to me -- thank you for taking the time to update this code to python3.
Are there any tests for ROSE2 that were run to ensure this doesn't break things or have significant negative effects on performance?
I'm very sorry that this wasn't reviewed sooner. Also, I'm not very familiar with the ROSE2 code. If you are interested in becoming a contributor on this repository to help maintenance of ROSE2, please let me know.
| import os | ||
| import subprocess | ||
| from string import join | ||
| # from string import join |
There was a problem hiding this comment.
Please delete code instead of commenting it out.
| pass | ||
|
|
||
| from string import join, maketrans | ||
| # from string import join, maketrans |
There was a problem hiding this comment.
please delete code instead of commenting it out
|
|
||
| # get the ID and convert to name | ||
| closestGene = startDict[allEnhancerGenes[distList.index(min(distList))]]['name'] | ||
| except: |
There was a problem hiding this comment.
Do you want this to fail silently? Or give some meaningful message to the user? Or catch a specific kind of error that you expect?
| idxStats= idxStats.communicate() | ||
| bamChromList = [line.split('\t')[0] for line in idxStats[0].split('\n')[0:-2]] | ||
| idxStats = subprocess.Popen(cmd, stdout=subprocess.PIPE, shell=True, | ||
| universal_newlines=True).communicate()[0] |
There was a problem hiding this comment.
Generally it's not good practice to use shell=True, but probably for a small scientific pipeline this would be okay. Here is an example of not using shell=True: https://github.com/HPC-buildtest/buildtest-framework/blob/devel/buildtest/utils/command.py#L55
| marker = idfun(item) | ||
| # in old Python versions: | ||
| # if seen.has_key(marker) | ||
| # if marker in seen |
There was a problem hiding this comment.
Is this necessary to keep since it's commented out?
|
@gauravj49 I suspect you'll need to rebase with master so that the fixes to the testing are present (and we can test your changes as well!) |
|
Hi all,
Just want to make sure you're connected with Jost who has been maintaining
ROSE2 for our lab.
Thanks for all of the work on bamliquidator!
…-Charles
Charles Y. Lin
On Wed, Mar 18, 2020 at 12:16 PM Vanessasaurus ***@***.***> wrote:
@gauravj49 <https://github.com/gauravj49> I suspect you'll need to rebase
with master so that the fixes to the testing are present (and we can test
your changes as well!)
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#67 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABC4BL5NOTEUK37Q34GN4LLRIDXUBANCNFSM4IEA7S2Q>
.
|
4 participants
No description provided.