Prepare the repoistory for zenodo

zavolanlab / bindz-rbp

RBP module for bindz, a bioinformatics tool to detect regulators' binding sites on RNA sequences.

https://github.com/zavolanlab/bindz-rbp

Apache License 2.0

7 stars 1 forks source link

Prepare the repoistory for zenodo #42

Closed AngryMaciek closed 4 years ago

AngryMaciek commented 4 years ago

Description

docs: update usage instructions
build: update ATtRACT database

Type of change

[x] Documentation updated

Checklist:

[x] My code follows the style guidelines of this project
[x] I have performed a self-review of my own code
[x] I have commented my code, particularly in hard-to-understand areas
[x] My changes generate no new warnings
[x] I have added tests that prove my fix is effective or that my feature works
[x] New and existing unit tests pass locally with my changes
[x] I have not reduced the existing code coverage

krish8484 commented 4 years ago

I ran the commands in readme, working perfectly!

AngryMaciek commented 4 years ago

@uniqueg Could you please take a look here? We have a very strange error related to the singularity version. Notice that all the tests pass on Travis CI - Branch. However, the exact same tests fail on Travis CI Pull Request. How is this possible?

@krish8484 Since you were the one who added singularity to the CI - could you please take a look as well? In recent days, after no significant changes to the code (I was just modifying the README) the CI started to fail due to errors in singularity version, which makes me believe that the installation itself is not stable... Take a look at the commit c220446 - the first one where the CI breaks, however, I have just corrected a typo there. I have now updated snakemake version, so that the erorr is now handled properly (please see logs) but I feel that the installation in the .travis might require a revisit...

AngryMaciek commented 4 years ago

@mzavolan : Could you please take a look at the updated README? @uniqueg , @krish8484 : Could you please take a look at the CI issue I described above?

krish8484 commented 4 years ago

Hi @AngryMaciek, I am looking into it.

uniqueg commented 4 years ago

The title and description of this PR are not very informative. From the changes it looks to me like two different things have been done:

docs: update usage instructions
build: update ATtRACT database

uniqueg commented 4 years ago

As for the CI issue: I have no clue why the tests fail on the PR check but not on the push check. It seems that Singularity isn't built correctly for the PR checks. Looking at the Travis config it's not obvious to me why. General info on building PR's is here, but I'm not sure it's helpful. But note that push checks are done on the feature branch and PR checks on merge commits against the target branch. So perhaps some conflict there? Doesn't notify of conflicts though.

Just FYI: I disable PR checks in all my projects (unless they are coming from forks). Just don't see why I would want to run checks twice. But apparently there are situations where the difference seems to matter. Still, never had any issues: if the puish checks passed my software was always behaving as expected. And I would expect the same here: if you run your tests locally, you probably don't have any issue.

krish8484 commented 4 years ago

I agree with @uniqueg about the push and PR checks. Moreover, I don't understand why the build is failing, the error says that the singularity version should be above 2.4.1, but we are downloading the zipped file of version 3.5.2, ideally there shouldn't be any problems. I can only think of one solution, i.e to remove the if condition which checks the version and throws the error but I dont know if it is the best step going forward.

AngryMaciek commented 4 years ago

I got an idea.

The CI started to crash after I have changed a typo in c220446, right?
So I have corrected another one in 4276f48 and now everything is fine.

Software engineering is indeed magical ✨

This issue is not linked to our tool at all - it is about a having a proper singularity installed if a user insists on the containers. Therefore I consider it fixed.

I am waiting for the final green light from Mihaela and the I merge to dev, master, put out a 1.0.0 version and we may upload to zenodo.