search

AnacondaRecipes / scikit-learn-feedstock

A conda-smithy repository for scikit-learn.
BSD 3-Clause "New" or "Revised" License
0 stars 2 forks source link

Fix v1.1.1 to build all variants #15

Closed pseudoyim closed 2 years ago

pseudoyim commented 2 years ago

Following my last PR, I found that osx-arm64 had all of the python variants built, but the other platforms only got the py310 variant built: Screen Shot 2022-07-18 at 5 23 08 PM

This behavior was being caused by the conda_build_config.yaml I added in the recipe, which I used to pin versions for python and numpy using zip_keys. I thought this was the best way to handle the version pinning requirements for v1.1.1.

But there may be a bug in how the zip_keys are handled when there is more than one conda_build_config.yaml (including the one in the aggregate root directory). So to work around this (per Serhii's recommendation), I'm using selectors to couple the appropriate numpy and python versions. The correct variants are now building for all platforms: Screen Shot 2022-07-20 at 2 56 00 PM

skupr-anaconda commented 2 years ago

An error on win64: 0 [main] rm (3884) C:\miniconda3\Library\usr\bin\rm.exe: *** fatal error - add_item ("\??\C:\miniconda3\Library", "/", ...) failed, errno 1

Something is wrong with git or bash/sh on Prefect

pseudoyim commented 2 years ago

@varlackc - The pinning for cython >=0.29.24 comes from here; I did notice the discrepancy with the pinning you linked to, but thought the tighter pinning was better to leave as is. That change was originally made in this PR.

The pinning for setuptools <60 comes from here: https://github.com/scikit-learn/scikit-learn/blob/1.1.1/pyproject.toml#L4

I'm not sure why the requirements for numpy you cited are relevant in this PR (which is for scikit-learn). Could you please elaborate?

varlackc commented 2 years ago

@varlackc - The pinning for cython >=0.29.24 comes from here; I did notice the discrepancy with the pinning you linked to, but thought the tighter pinning was better to leave as is. That change was originally made in this PR.

The pinning for setuptools <60 comes from here: https://github.com/scikit-learn/scikit-learn/blob/1.1.1/pyproject.toml#L4

I'm not sure why the requirements for numpy you cited are relevant in this PR (which is for scikit-learn). Could you please elaborate?

Thanks Paul for the reply, that makes sense, Just wanted to verify that we had some consistency between the requirements for the package and the dependency.