Skip to content

Conversation

@jakirkham
Copy link
Member

@jakirkham jakirkham commented Oct 31, 2018

Fixes #86

Went ahead and used the latest release of Cython (0.29) to regenerate the C files here. Not sure if there is a version preference, but happy to update if needed. Should move us toward Python 3.7 support.

Also there was a vlen.html file in here, which I went ahead and dropped as we didn't have HTML files for the other Cython sources. Updated .gitignore to keep these from sneaking back in again. ;) Though if I missed something and this needs to be reverted, happy to do so.

TODO:

  • Unit tests and/or doctests in docstrings
  • tox -e py36 passes locally
  • tox -e py27 passes locally
  • Docstrings and API docs for any new/modified user-facing classes and functions
  • Changes documented in docs/release.rst
  • tox -e docs passes locally
  • AppVeyor and Travis CI passes
  • Test coverage to 100% (Coveralls passes)

Guessing we don't need this in the source as HTML files are not
generated for the other Cython modules, so dropping this in an effort to
tidy things up here.
Use the latest Cython to regenerate C files for distribution. These
should include support for Python 3.7 as well, which will be useful as
we add Python 3.7 support to Numcodecs generally.
@alimanfoo
Copy link
Member

Thanks @jakirkham.

This would probably be a good time to also upgrade the packages specified in requirements_dev.txt, so versions used in our dev and CI environment are in line with version of cython used to generate the C sources, and bring everything else up to date too.

When I upgraded requirements_dev.txt recently in the zarr repo, I started from a blank environment, then used pip to install the latest version of all the packages that are imported anywhere in the zarr codebase, then did a pip freeze and dumped out to requirements_dev.txt, then moved lmdb and bsddb3 to requirement_dev_optional.txt (because these are only run under linux CI).

Probably also a good idea if pinned versions of packages in requirements_dev.txt are the same between zarr and numcodecs projects, although the set of packages required is not exactly the same (i.e., some packages are needed for zarr and not strictly needed for numcodecs). Not sure what is the best way to achieve that.

There is also a security warning currently regarding the version of the requests package in the requirements_dev.txt on the zarr repo. Doesn't look serious but would be nice to make it go away.

Maybe we should update requirements_dev.txt on both repos at the same time? Any other suggestions for how to manage the process of upgrading the pinned dev/CI requirements?

@jakirkham
Copy link
Member Author

Updating the requirements sounds like a good idea.

Would recommend enlisting a bot to do this for us to keep the maintenance simple. Basically these send PRs periodically to update any dependencies that are out-of-date.

One of the common ones along these lines I've seen is PyUp. Looks like GitHub's Marketplace has a few other options with a couple of them noting Python support.

WDYT?

@alimanfoo
Copy link
Member

A bot sounds cool, but what's a bit complicated is that requirements_dev.txt currently pins everything in the environment, and some of those are indirect dependencies, i.e., not directly imported by zarr or numcodecs. In upgrading a direct dependency, that package may add or drop completely an indirect dependency, and I don't think a bot would be able to capture that type of change. Also if a bot is just bumping version numbers, but there are some incompatibilities expressed in min or max versions for other packages, that would not get captured.

What I think we need is a way to specify the direct dependencies, then a way to generate a complete list of pinned packages that would be obtained by installing the latest versions of direct dependencies into an environment. Then redo that procedure periodically when new versions of direct dependencies become available.

Btw I think it is a good thing to be listing and pinning everything in requirements_dev.txt, not just direct dependencies, so we get fully reproducible builds.

I guess the alternative would be to list only direct dependencies in requirements_dev.txt, and let indirect dependencies float free. Then we could use a bot like PyUp.

@alimanfoo
Copy link
Member

I guess the alternative would be to list only direct dependencies in requirements_dev.txt, and let indirect dependencies float free. Then we could use a bot like PyUp.

Just to add this makes me slightly nervous, but would make maintenance much simpler, especially if we can use a bot. Tempted. WDYT?

@jakirkham
Copy link
Member Author

Yeah I think you're right that the bot probably won't pick up indirect dependencies. Historically PyPI hasn't provided enough data to make this reasonable (mostly because there are old packages that don't supply this information either and still are on PyPI).

Incompatibilities are of course also hard. Though even pip doesn't have a dependency resolver. ( pypa/pip#988 ) So maybe the bot doesn't make this much harder than it already is.

Yeah, can definitely see the value in having a full pip freeze. Maybe one thing we can do to aid this is starting to run pip freeze in CI builds. The value being that if new dependencies pop-up during one of these upgrades we can quickly copy them out of the logs and tack them onto the bot PR.

Dropping indirect dependencies is certainly another option. There's at least one case where this has been helpful for us. ( zarr-developers/zarr-python#126 ) So maybe this is a good option after all.

On a totally unrelated note, we could set up daily cron jobs of the CIs to be notified when anything goes off the rails. Maybe this helps alleviate one underlying concern motivating this discussion.

@jakirkham
Copy link
Member Author

At least as far as this PR is concerned, we probably don't need to address all of these issues. Though I agree they are things we should address and ideally soon. For now, have bumped the Cython dependency to match what I used to regenerate the C files.

@alimanfoo
Copy link
Member

alimanfoo commented Nov 1, 2018 via email

@jakirkham
Copy link
Member Author

Have broken out the requirement refreshing ( #91 ) and using a bot to manage requirements ( zarr-developers/zarr-python#322 ) as issues.

@jakirkham jakirkham merged commit 8eabe18 into zarr-developers:master Nov 2, 2018
@jakirkham jakirkham deleted the cython_0.29_regenerate_c_files branch November 2, 2018 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants