-
Notifications
You must be signed in to change notification settings - Fork 37
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
Update readme to reflect stable install #391
Conversation
@sssangha Is there a reason we fall back to the past approach? |
@sssangha lets disable the test for the water mask? |
+1 |
The issue here is that the README does not describe a working process to create a repeatable env in which to run this software. Specifically, the line Before I can implement any testing suites I actually need to be able to replicate Simran's results on my machine with my env, as clearly what I have now fails all my tests. To do that we need a way to create envs for this software that work and give the same outputs when provided the same inputs and aria-tools software. Maybe your requirements file needs work to make this happen. |
@dbekaert and @bbuzz31 , I was able to confirm from Alex's suggestion that mamba works when specifying it can be found through Will have to still figure out what's going wrong when using |
When the
|
@cmarshak , agreed went ahead and adjusted the readme to reflect your suggestion. Specifically, install the package using miniforge |
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.
Can you also fix this line:
setenv PATH ${PATH}:${PWD}/tools/ARIAtools
which should be
setenv PATH ${PATH}:${PWD}/tools/bin
done |
Hi @sssangha, this looks good. However, I think we also need to add the |
Sure, done. I tested the new environment with these packages included and all looks good. The golden outputs I just shared for |
83dfa4f
to
0cbd0be
Compare
Requirements file and mamba no longer translate to a stable build. Updating readme to reflect this