Update conda recipe and requirements #305

jbiggsets · 2019-10-09T14:06:00Z

This PR is intended to address #297, but it includes several additional changes that should to be reviewed closely.

Here is a quick rundown of the changes:

Removed conda_requirements.txt. Previously, we were maintaining two sets of requirements: requirements.txt and conda_requirements.txt. The second was being used to create conda environments (using conda create -n foo --file conda_requirements.txt). However, since these sets of requirements were virtually identical and both can be used to create conda environments, I discarded the conda_requirements.txt file. The one major difference between these files is that conda_requirements.txt had zeromq. This cannot be in the requirements.txt file, since it leads to an error when using pip install -e .. I think this is all right, since it's not entirely necessary (and may be installed by Jupyter anyway?), but we can add it to the meta.yaml separately, if it's a problem. (See below on the updates to meta.yaml.)
Switched to requirements.txt in .travis.yml. Related to the above, our travis.yml file had previously used conda_requirements.txt to set up the conda environment for our builds. This has been changed to use requirements.txt in our Travis build.
Updated requirements.txt. Some minor tweaks were also made to the requirements.txt file, mostly reordering the list of requirements. I also removed scikit-learn (since this is already a skll dependency).
Updated meta.yaml to use setup.py. Previously, the meta.yaml file was listing all of the requirements manually, which meant we had to ensure these requirements were always up-to-date. I changed this configuration file to instead rely on the setup.py data, and pull the requirements directly from install_requires. This update takes advantage of Jinja templating. I tested this on my Mac, and it worked. I am going to test it on a Windows machine today, and it would probably be good to have others test it on their machines, as well.
Re-generated environment.yml. Finally, I re-generated the environment.yml to ensure that it was consistent with our requirements.txt file.
Add missing test file. Added test_container.py to .travis.yml and Azure, per test_container.py missing in both .travis.yml and azure-pipelines.yml #310.

…rted): - Clean up requirements.txt file - Clean up environment.yml file - Remove conda_requirements.txt file - Update travix.yml file to use requirements.txt - Update meta.yml to use setup.py

coveralls · 2019-10-09T14:18:20Z

Coverage increased (+0.6%) to 91.643% when pulling 6b30388 on update-conda-recipe into a8f674f on master.

hlepp

We should update the instructions for installation here:
doc/contributing.rst

desilinguist

I wasn't sure why come changes were made.

desilinguist · 2019-10-10T15:31:38Z

conda-recipe/rsmtool/meta.yaml

-    - xlrd
-    - xlwt
-
+    - python 3.6.8


I think it might be nice to build noarch packages that can still pin Python versions. For example, see here. Also, I don't think we want to pin to a specific version of Python 3.6. Any Python >= 3.6 should work fine, including Python 3.7 right?

I agree with everything, except I wasn't sure about 3.7. I've never tested anything with 3.7, and the old conda_requirements.txt had python>=3.6.3,<3.7.

desilinguist · 2019-10-10T15:31:52Z

conda-recipe/rsmtool/meta.yaml

-    - openpyxl
-    - xlrd
-    - xlwt
+    - python 3.6.8


desilinguist · 2019-10-10T15:32:09Z

environment.yml

- openpyxl
- xlrd
- xlwt
+  - python=3.6.8


Again, let's not be too specific here.

desilinguist · 2019-10-10T15:33:11Z

environment.yml

+  - matplotlib=2.1.2
+  - nose=1.3.7
+  - notebook=5.7.2
+  - numpy=1.14.0


Do we need to pin numpy? Since this is only used for readthedocs, using the latest version of numpy should be fine right? And same for pandas etc.?

desilinguist · 2019-10-10T15:33:18Z

requirements.txt

@@ -4,18 +4,17 @@ joblib==0.11
 matplotlib==2.1.2
 nose==1.3.7
 notebook==5.7.2
-numpy==1.14.*
+numpy==1.14.0


Why this change?

I don't have strong feelings about this, but I'm always paranoid about differences in even minor version of numpy leading to small differences in results.

desilinguist · 2019-10-10T15:33:43Z

requirements.txt

@@ -4,18 +4,17 @@ joblib==0.11
 matplotlib==2.1.2
 nose==1.3.7
 notebook==5.7.2
-numpy==1.14.*
+numpy==1.14.0
 pandas==0.23.4


Do things break if we use the latest version of pandas?

Not sure. I'll test it out. :)

Yes, they do.

aloukina

The changes look good to me. I would probably concur with Nitin on not pinning to too specific versions to avoid conflicts with other tools since most of the computations we do are not too complex.

desilinguist · 2019-10-28T19:39:06Z

@jbiggsets what's the status of this PR?

jbiggsets · 2019-10-28T20:03:17Z

@jbiggsets what's the status of this PR?

Looks like I have a few minor comments to address still. I'll let you know when it's ready.

desilinguist

One more minor change.

desilinguist · 2019-11-01T18:37:17Z

conda-recipe/rsmtool/meta.yaml

@@ -9,7 +11,7 @@ build:
  number: 0
  script:
    - cd $SRC_DIR
-    - $PYTHON setup.py install
+    - $PYTHON setup.py install --single-version-externally-managed --record=record.txt


I think you don't need to do this since its use is discouraged. See the SKLL recipe for a much easier alternative.

desilinguist · 2019-11-01T18:41:36Z

We should also check that we aren't bundling test data (which tends to be large) into our conda or PyPI package.

jbiggsets · 2019-11-01T19:04:43Z

We should also check that we aren't bundling test data (which tends to be large) into our conda or PyPI package.

Why would that be the case, and how would we check that?

desilinguist · 2019-11-01T19:06:12Z

Why would that be the case, and how would we check that?

In SKLL, when I just did python setup.py install, it included tests, test data, examples, and example data in the conda package. I think we prevented it in SKLL, through a combination of the MANIFEST.in file and through appropriate fields in setup.py.

…irements

The installation instructions will be updated in an upcoming PR.

desilinguist · 2019-11-21T00:36:55Z

The conda package was tested and worked fine on both Linux and Windows as seen here. Therefore, I am now merging this PR.

…ingService/update-conda-recipe Update conda recipe and requirements

jbiggsets added 6 commits September 25, 2019 15:57

This commit includes the following changes (some of which may be reve…

43bca49

…rted): - Clean up requirements.txt file - Clean up environment.yml file - Remove conda_requirements.txt file - Update travix.yml file to use requirements.txt - Update meta.yml to use setup.py

remove zeromq from requirements

079fb34

Removing sklearn

b3a784b

Merge branch 'master' into update-conda-recipe

810aae2

Making python 3.6.8 in meta.yaml

a6a2243

Adding zeromq back to requirements, and updating environemtn.yml

7236afb

jbiggsets added the enhancement label Oct 9, 2019

jbiggsets requested review from desilinguist and aloukina October 9, 2019 14:06

removing zeromq

cb878ef

jbiggsets requested a review from hlepp October 9, 2019 14:26

hlepp previously requested changes Oct 9, 2019

View reviewed changes

desilinguist requested changes Oct 10, 2019

View reviewed changes

jbiggsets added 3 commits October 16, 2019 10:15

Various updates to address PR comments

816c97d

trying to fix travis

25f7c29

reverting back to the older version of pandas

8ce9461

aloukina approved these changes Oct 16, 2019

View reviewed changes

jbiggsets and others added 4 commits October 31, 2019 12:59

Additional updates to requirements, travis, and environment

2b36bf2

Merge branch 'master' into update-conda-recipe

55e125e

Merge branch 'master' into update-conda-recipe

729967a

Adding test_container to azure

feedf41

desilinguist requested changes Nov 1, 2019

View reviewed changes

jbiggsets added 2 commits November 4, 2019 15:38

Updating MANIFEST and meta.yaml

6f6a647

Merge branch 'master' into update-conda-recipe

16f34c3

jbiggsets added 3 commits November 19, 2019 11:55

Fixing some issues in conda recipe, and unpinning most things in requ…

c316ea0

…irements

updating version and trying to pin numpy to 14.6

ffb5727

Trying to fix a problem in the notebooks with tight_layout

22d648d

desilinguist approved these changes Nov 19, 2019

View reviewed changes

updating model files

6b30388

desilinguist merged commit 852649c into master Nov 21, 2019

delete-merged-branch bot deleted the update-conda-recipe branch November 21, 2019 00:37

This was referenced Nov 21, 2019

test_container.py missing in both .travis.yml and azure-pipelines.yml #310

Closed

Generate requirements in meta.yaml from setup.py? #297

Closed

srhrshr pushed a commit to srhrshr/rsmtool that referenced this pull request Oct 23, 2021

Merge pull request EducationalTestingService#305 from EducationalTest…

741c78e

…ingService/update-conda-recipe Update conda recipe and requirements

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update conda recipe and requirements #305

Update conda recipe and requirements #305

jbiggsets commented Oct 9, 2019 •

edited

coveralls commented Oct 9, 2019 •

edited

hlepp left a comment

desilinguist left a comment

desilinguist Oct 10, 2019

jbiggsets Oct 10, 2019

desilinguist Oct 10, 2019

desilinguist Oct 10, 2019

desilinguist Oct 10, 2019

desilinguist Oct 10, 2019

jbiggsets Oct 10, 2019

desilinguist Oct 10, 2019

jbiggsets Oct 10, 2019

jbiggsets Oct 31, 2019

aloukina left a comment

desilinguist commented Oct 28, 2019

jbiggsets commented Oct 28, 2019

desilinguist left a comment

desilinguist Nov 1, 2019

desilinguist commented Nov 1, 2019

jbiggsets commented Nov 1, 2019

desilinguist commented Nov 1, 2019 •

edited

desilinguist commented Nov 21, 2019

Update conda recipe and requirements #305

Update conda recipe and requirements #305

Conversation

jbiggsets commented Oct 9, 2019 • edited

coveralls commented Oct 9, 2019 • edited

hlepp left a comment

Choose a reason for hiding this comment

desilinguist left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aloukina left a comment

Choose a reason for hiding this comment

desilinguist commented Oct 28, 2019

jbiggsets commented Oct 28, 2019

desilinguist left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

desilinguist commented Nov 1, 2019

jbiggsets commented Nov 1, 2019

desilinguist commented Nov 1, 2019 • edited

desilinguist commented Nov 21, 2019

jbiggsets commented Oct 9, 2019 •

edited

coveralls commented Oct 9, 2019 •

edited

desilinguist commented Nov 1, 2019 •

edited