Use pip-compile for a lockfile of constraints #2625

richafrank · 2020-01-21T17:36:18Z

requirements.txt is now that constraints file (used by CI). requirements.in and other *.in have top-level dependencies, which are read by setup.py. setup.py no longer re-writes == to >=. It's built from all the *.in files, i.e. including dev/test requirements.
Removed cyordereddict recipe that we no longer depend on
Updated to newer versions of dependencies in CI env
Also has a potential fix for appveyor not uploading conda packages from master currently, but won't know if it works until the fix is on master

coveralls · 2020-01-21T17:56:06Z

Coverage decreased (-0.006%) to 88.268% when pulling 23f1580 on pip-compiled into 74010a8 on master.

ssanderson

@richafrank I took a pass here. I had a bunch of observations that are mostly further improvements we could make in the future, but I think most of them are strictly additive, so we shouldn't block on them.

One thing I'm not sure about is how these changes interact with our existing conda machinery. Do you have a high-level overview of whether/how this affects our conda install situation?

ssanderson · 2020-01-22T21:19:39Z

etc/requirements.in

@@ -0,0 +1,63 @@
+# Incompatible with earlier PIP versions


Do we still actually need pip and setuptools here?

I can investigate - I haven't looked into it. I think this is no worse than the current situation, so I'd love to merge as is and then make further improvements.

I looked into this and removed them in #2610: they are not necessary. (That PR is failing for unrelated reasons, which this one should help to fix.)

+1 on just leaving them here for now though.

ssanderson · 2020-01-22T21:21:39Z

etc/requirements.in

+numexpr>=2.6.1
+
+# Currency Codes
+iso4217>=1.6.20180829


Definitely not a blocker for merging, but another option for the content of this file would be to move it canonically into the install_requires of zipline's setup.py, which would reduce the need for parsing/manipulating this file.

What's your thought on what input we would give to pip-compile in lieu of requirements.in?

I think then we could just use ..

Ah, you're suggesting moving the list of libs to setup.py and making requirements.in just ..

Since we need both requirements.in and setup.py and want one to refer to the other, my preference is to stick with them being listed in requirements.in. Then we can keep the various extras split up in separate files, and have the files well commented, instead of inlining all that in setup.py.

ssanderson · 2020-01-22T21:22:36Z

etc/requirements_build.in

@@ -0,0 +1,2 @@
+Cython>=0.25.2


I think a good next step after merging this would be to move these into a pyproject.toml.

ssanderson · 2020-01-22T21:23:32Z

etc/requirements_dev.in

+# Linting
+flake8>=3.3.0
+
+# Algo examples


Not a blocker, but it seems unfortunate to have matplotlib in here. The rest of the dev requirements are pretty lightweight, but mpl requires a bunch of external dependencies. We might want to consider just making mpl fully optional, since none of our tests actually use it.

Yep, I buy that. I didn't want to make changes in this PR bigger than making the process easier, and then I figure we can iterate on making us more efficient.

ssanderson · 2020-01-22T21:27:47Z

setup.py

@@ -256,11 +227,10 @@ def extras_requires(conda_format=False):
    return extras


-def setup_requirements(requirements_path, module_names, strict_bounds,


Do we still need the strict_bounds machinery for the conda build case? It seems like that's what this was used for originally. I don't have a good sense of how these changes interact with our existing conda machinery.

I don't believe we need it anymore. Let's look at the different cases:

-- install_requires when invoked by pip --
Expected output is to give wide bounds (i.e. >=). We did that by rewriting requirements.txt's ==, so we never passed strict_bounds at the call site, using the default value of False.
This is now accomplished by reading requirements.in, which itself uses wide bounds.
A benefit now is that we don't include our transitive dependencies, which makes the install more flexible.

-- install_requires when invoked by conda-build --
Same as pip case above.

-- setup_requires when invoked by pip --
strict_bounds was False in this case, so you'd get latest numpy/cython, which is the whole issue with needing to install the expected numpy version in a separate, earlier step. I don't believe it actually matters for cython, but correct me if I'm wrong. This behavior hasn't changed, now that we read from requirements_build.in without rewriting.

-- build_requires when invoked by conda-build --
strict_bounds was True in this case, but so was conda_format, which overrides any numpy version with x.x, so that it's pinned to the version that we specified to conda-build. This way, it will be built with the same version as it requires for install, and the package is tagged with the version. Again, I don't think the behavior for cython matters, but this is a difference here, in that it used to be strict and is no longer. No longer having both strict_bounds and conda_format as ways to rewrite our requirements is a vast improvement.

-- extras_require when invoked by pip --
Previously this used strict_bounds=True, since the extras were mostly dev requirements, and the thought was to pin them exactly for consistent dev environments. Now this reads from requirements_dev.in, i.e. just lower bounds, and if you want to match the CI environment, you can install using the constraints.

-- extras_require when invoked by conda-build --
Our conda recipe for zipline doesn't use the extras_require.

I think those all make sense to me. Thanks for the rundown.

richafrank

@ssanderson Thanks for the review. Take a look at my response to your setup.py question about conda-build, and let me know if that doesn't clarify your general conda install question. The net impact should be just that cython isn't pinned at build time, and we don't specify all our transitive dependencies anymore.

richafrank · 2020-01-23T17:48:40Z

etc/requirements.in

@@ -0,0 +1,63 @@
+# Incompatible with earlier PIP versions


I can investigate - I haven't looked into it. I think this is no worse than the current situation, so I'd love to merge as is and then make further improvements.

richafrank · 2020-01-23T17:54:23Z

etc/requirements.in

+numexpr>=2.6.1
+
+# Currency Codes
+iso4217>=1.6.20180829


What's your thought on what input we would give to pip-compile in lieu of requirements.in?

richafrank · 2020-01-23T17:56:02Z

etc/requirements_dev.in

+# Linting
+flake8>=3.3.0
+
+# Algo examples


Yep, I buy that. I didn't want to make changes in this PR bigger than making the process easier, and then I figure we can iterate on making us more efficient.

richafrank · 2020-01-23T18:36:38Z

setup.py

@@ -256,11 +227,10 @@ def extras_requires(conda_format=False):
    return extras


-def setup_requirements(requirements_path, module_names, strict_bounds,


I don't believe we need it anymore. Let's look at the different cases:

-- install_requires when invoked by pip --
Expected output is to give wide bounds (i.e. >=). We did that by rewriting requirements.txt's ==, so we never passed strict_bounds at the call site, using the default value of False.
This is now accomplished by reading requirements.in, which itself uses wide bounds.
A benefit now is that we don't include our transitive dependencies, which makes the install more flexible.

-- install_requires when invoked by conda-build --
Same as pip case above.

-- setup_requires when invoked by pip --
strict_bounds was False in this case, so you'd get latest numpy/cython, which is the whole issue with needing to install the expected numpy version in a separate, earlier step. I don't believe it actually matters for cython, but correct me if I'm wrong. This behavior hasn't changed, now that we read from requirements_build.in without rewriting.

-- build_requires when invoked by conda-build --
strict_bounds was True in this case, but so was conda_format, which overrides any numpy version with x.x, so that it's pinned to the version that we specified to conda-build. This way, it will be built with the same version as it requires for install, and the package is tagged with the version. Again, I don't think the behavior for cython matters, but this is a difference here, in that it used to be strict and is no longer. No longer having both strict_bounds and conda_format as ways to rewrite our requirements is a vast improvement.

-- extras_require when invoked by pip --
Previously this used strict_bounds=True, since the extras were mostly dev requirements, and the thought was to pin them exactly for consistent dev environments. Now this reads from requirements_dev.in, i.e. just lower bounds, and if you want to match the CI environment, you can install using the constraints.

-- extras_require when invoked by conda-build --
Our conda recipe for zipline doesn't use the extras_require.

willianpaixao · 2020-01-28T14:39:50Z

I know this PR is pretty much close to merge, but why aren't we using Pipenv instead of pip-tools?

ssanderson · 2020-01-29T14:10:04Z

@willianpaixao we've experimented quite a bit with pipenv internally at Quantopian and found that it wasn't stable enough for our needs.

Removed cyordereddict recipe that we no longer depend on Removed strict bounds rewriting in setup.py. Now setup.py reads from requirements.in, which is not strict in its bounds Need to update case for pip-compile's output Filter out comments from lockfile, now that we have "via"

Required setup.py handling a requirements without a spec

since it's the generated lockfile

ssanderson reviewed Jan 22, 2020

View reviewed changes

richafrank commented Jan 23, 2020

View reviewed changes

richafrank force-pushed the pip-compiled branch from 26092f5 to ab34b1a Compare January 24, 2020 16:11

richafrank force-pushed the pip-compiled branch 3 times, most recently from 110b5ff to c6b44e8 Compare February 7, 2020 01:22

richafrank added 8 commits February 11, 2020 11:05

MAINT: Removing unused, stale requirements file

6cf1e62

MAINT: Updated to newer versions of dependencies

66ac29c

BLD: Potential fix for appveyor not uploading conda packages from master

0e8fd07

DOC: On updating dependencies

4f78b86

DEV: Added pre-commit to dev reqs

0dc7d39

Required setup.py handling a requirements without a spec

MAINT: Remove last remnants of ordered_pip

555d1df

MAINT: Renamed requirements.txt to requirements_locked.txt

23f1580

since it's the generated lockfile

richafrank force-pushed the pip-compiled branch from 7436b5d to 23f1580 Compare February 11, 2020 16:15

richafrank merged commit e5b0135 into master Feb 11, 2020

richafrank deleted the pip-compiled branch February 11, 2020 22:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use pip-compile for a lockfile of constraints #2625

Use pip-compile for a lockfile of constraints #2625

richafrank commented Jan 21, 2020 •

edited

Loading

coveralls commented Jan 21, 2020 •

edited

Loading

ssanderson left a comment

ssanderson Jan 22, 2020

richafrank Jan 23, 2020

quantophred Jan 28, 2020

ssanderson Jan 22, 2020

richafrank Jan 23, 2020

ssanderson Jan 23, 2020

richafrank Jan 23, 2020

ssanderson Jan 22, 2020

ssanderson Jan 22, 2020

richafrank Jan 23, 2020

ssanderson Jan 22, 2020

richafrank Jan 23, 2020 •

edited

Loading

ssanderson Jan 23, 2020

richafrank left a comment

richafrank Jan 23, 2020

richafrank Jan 23, 2020

richafrank Jan 23, 2020

richafrank Jan 23, 2020 •

edited

Loading

willianpaixao commented Jan 28, 2020

ssanderson commented Jan 29, 2020

		@@ -256,11 +227,10 @@ def extras_requires(conda_format=False):
		return extras


		def setup_requirements(requirements_path, module_names, strict_bounds,

Use pip-compile for a lockfile of constraints #2625

Use pip-compile for a lockfile of constraints #2625

Conversation

richafrank commented Jan 21, 2020 • edited Loading

coveralls commented Jan 21, 2020 • edited Loading

ssanderson 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

Choose a reason for hiding this comment

richafrank Jan 23, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

richafrank 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

richafrank Jan 23, 2020 • edited Loading

Choose a reason for hiding this comment

willianpaixao commented Jan 28, 2020

ssanderson commented Jan 29, 2020

richafrank commented Jan 21, 2020 •

edited

Loading

coveralls commented Jan 21, 2020 •

edited

Loading

richafrank Jan 23, 2020 •

edited

Loading

richafrank Jan 23, 2020 •

edited

Loading