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

merge snakemake rules and add msa_size info to index #67

Merged
merged 2 commits into from Sep 26, 2017

Conversation

tkosciol
Copy link
Member

addressing #48 and #52

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 88.569% when pulling ea48820 on merge_snakemake_rules into dc411f2 on master.

Copy link
Contributor

@sjanssen2 sjanssen2 left a comment

Choose a reason for hiding this comment

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

I have not executed the pipeline in my machine but this PR looks good to me from reading the code.

@@ -107,130 +107,115 @@ def split_x(inp_0, out_0, step="NA", version=config['VERSION'],
shell('echo "pass" > {out_0}')


def copy_out(source_dir='01-PDB', dest_dir='PDB', tempdir='/dev/null'):
Copy link
Contributor

Choose a reason for hiding this comment

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

do we want to copy or can we move to same space? How about softlinks to avoid redundancy?

Copy link
Member Author

Choose a reason for hiding this comment

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

this function is used to copy from localscratch to the main filesystem, hence the use of rsync to package the data on its way.
so softlinks are a no-go and move = copy+delete, hence it's not needed because I do a bulk clean-up later on.

@sjanssen2
Copy link
Contributor

It would be nice to have unit tests for the snakemake_helper functions!

@tkosciol
Copy link
Member Author

agree re unit tests. I need to do this soon and will issue a separate PR to address.

@qiyunzhu qiyunzhu merged commit 1988897 into master Sep 26, 2017
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.

None yet

4 participants