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

Update and sync dependencies in `requirements.txt` and `environment.yml` #350

Merged
merged 8 commits into from Nov 28, 2019

Conversation

@rossbar
Copy link
Contributor

rossbar commented Nov 22, 2019

This PR addresses issue #349 by:

  • updating the versions of dependencies in both environment.yml and requirements.txt so that they are consistent with each other.
  • Verifying that creating the environment works using either conda or the venv module.
  • Updating the README with a few additional instructions on how to create the environment with using venv (for non-conda users).

Currently the conda-ish way of setting up the environment isn't tested at all in the CI. I don't have any experience with wercker, but it seems like adding the following to wercker.yml under the build pipeline might do the trick:

    - script:
        name: test conda build
        code: |
          conda env create --name elegant-scipy -f environment.yml

I didn't add it because I don't want to break the CI for the repo. Let me know if this is something that would be of interest.

rossbar added 4 commits Nov 22, 2019
Changed the pinned versions of packages in requirements.txt
to those that result from installation in a new venv.
Verified environment.yml yields a functional working environment
locally (conda 4.7.12, python 3.7.5).
* Added a sub-section to the README on dependency installation to
  include instructions for setting up a virtual environment using
  the built-in venv module (for non-conda users)

* Updated .gitignore to automatically ignore venv/ folder
@jni

This comment has been minimized.

Copy link
Collaborator

jni commented Nov 22, 2019

This looks great, thanks @rossbar! I must admit I'm not sure about Wercker but it's worth adding that and seeing if it doesn't break. If it does break, you can always revert that commit!

Thanks!

rossbar added 3 commits Nov 22, 2019
This reverts commit 3c32cf5.
@rossbar

This comment has been minimized.

Copy link
Contributor Author

rossbar commented Nov 22, 2019

Ok, I think this is ready for review.

I tried modifying the wercker CI but my approach did not work. I reverted and ended up having to modify the pip-install step in the wercker build to force pip to re-install everything. At least the CI is working again, though it is still not testing the constructing of the conda environment

Modified dependencies files to remove bs4 alias as per #331.
@stefanv

This comment has been minimized.

Copy link
Collaborator

stefanv commented Nov 28, 2019

Do we still want the force option in there? Otherwise looks good, thanks!

@jni jni merged commit 2473d74 into elegant-scipy:master Nov 28, 2019
1 check failed
1 check failed
wercker/build Wercker pipeline failed
Details
@jni

This comment has been minimized.

Copy link
Collaborator

jni commented Nov 28, 2019

@stefanv worst case, it means there is no caching used on Werker envs, so I think it's fine to merge as is. We can revert if it causes trouble. Thanks again @rossbar!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.