Adding elapid #18896

earth-chris · 2022-05-10T23:40:16Z

Checklist

Title of this PR is meaningful: e.g. "Adding my_nifty_package", not "updated meta.yaml".
License file is packaged (see here for an example).
Source is from official source.
Package does not vendor other packages. (If a package uses the source of another package, they should be separate packages or the licenses of all packages need to be packaged).
If static libraries are linked in, the license of the static library is packaged.
Build number is 0.
A tarball (url) rather than a repo (e.g. git_url) is used in your recipe (see here for more details).
GitHub users listed in the maintainer section have posted a comment confirming they are willing to be listed there.
When in trouble, please check our knowledge base documentation before pinging a team.

conda-forge-linter · 2022-05-10T23:40:28Z

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/elapid) and found it was in an excellent condition.

conda-forge-linter · 2022-05-11T00:04:23Z

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipes/elapid) and found some lint.

Here's what I've got...

For recipes/elapid:

Selectors are suggested to take a <two spaces>#<one space>[<expression>] form. See lines [13]
noarch packages can't have selectors. If the selectors are necessary, please remove noarch: python.

conda-forge-linter · 2022-05-11T00:08:33Z

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipes/elapid) and found some lint.

Here's what I've got...

For recipes/elapid:

Non noarch packages should have python requirement without any version constraints.
Non noarch packages should have python requirement without any version constraints.

conda-forge-linter · 2022-05-11T00:20:41Z

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/elapid) and found it was in an excellent condition.

earth-chris · 2022-09-15T18:44:06Z

@conda-forge-admin, please ping team

conda-forge-linter · 2022-09-15T18:44:08Z

Hi! This is the friendly automated conda-forge-webservice.

I was asked to ping @conda-forge/staged-recipes and so here I am doing that.

recipes/elapid/meta.yaml

saraedum · 2022-09-19T19:25:23Z

recipes/elapid/meta.yaml

+    - python
+    - descartes >=1.1.0
+    - geopandas >=0.7.0
+    - glmnet  # [py<39 and not win]


This seems to be the only thing here that makes this not a noarch: python recipe. Could you explain what this is about exactly? I.e., how come there is a dependency that is only needed when Python is old? (Might also be nice to add a comment to the recipe so contributors to the recipe in the future do not stumble upon this.

it's mostly because the glmnet package doesn't include a windows build. I was under the impression from this issue that python 3.9 and above was also unavailable. After checking the conda-forge page that doesn't appear to be the case, so it looks like this could just be [not win].

this is a statistical modeling package that has some nice-to-have features that are unavailable in sklearn. elapid works around this by using glmnet if it's installed, sklearn otherwise. I could technically drop the requirement entirely, but imho it's worth having those nice-to-haves.

@earth-chris FYI I did follow up with @civisanalytics to be able to maintain glmnet project; No one responded tho

yea, i saw that, thanks @amirhessam88. seems unlikely that they're gonna resume maintenance on the package, unfortunately, so future updates would probably have to come from someone who forks it and pushes a new package.

recipes/elapid/meta.yaml

saraedum

Looks alright to me. Please have a look at the comments above.

removed <py37 restriction, added dev_url Co-authored-by: Julian Rüth <[email protected]>

earth-chris · 2022-09-20T00:15:00Z

hi @saraedum, thanks so much for your review! I've accepted the edits you recommended and relaxed the python version constraints on glmnet (but kept the windows constraint; see package page for reference if needed). I just have one outstanding question clarifying your comments on major versions.

earth-chris · 2022-09-27T03:48:06Z

@conda-forge-admin, please ping team

conda-forge-linter · 2022-09-27T03:48:07Z

Hi! This is the friendly automated conda-forge-webservice.

I was asked to ping @conda-forge/staged-recipes and so here I am doing that.

saraedum · 2022-09-28T16:23:12Z

recipes/elapid/meta.yaml

+    - descartes >=1.1.0
+    - geopandas >=0.7.0
+    - glmnet >=2.0,<2.3  # [not win] windows build unavailable; elapid will use sklearn features instead
+    - numpy >=1.18
+    - pandas >=1.0.3
+    - rasterio >=1.2.1
+    - rtree >=0.9
+    - scikit-learn >=0.22.2
+    - shapely >=1.7.0
+    - tqdm >=4.60.0


Suggested change

- descartes >=1.1.0

- geopandas >=0.7.0

- glmnet >=2.0,<2.3 # [not win] windows build unavailable; elapid will use sklearn features instead

- numpy >=1.18

- pandas >=1.0.3

- rasterio >=1.2.1

- rtree >=0.9

- scikit-learn >=0.22.2

- shapely >=1.7.0

- tqdm >=4.60.0

- descartes >=1.1.0,<2

- geopandas >=0.7.0,<0.8

- glmnet >=2.0,<2.3 # [not win] windows build unavailable; elapid will use sklearn features instead

- numpy >=1.18

- pandas >=1.0.3,<2

- rasterio >=1.2.1,<2

- rtree >=0.9,<0.10

- scikit-learn >=0.22.2,<0.23

- shapely >=1.7.0,<2

- tqdm >=4.60.0,<5

Assuming that these all follow semantic versioning. They probably don't but pins can still be relaxed later.

ok, I've added upper bounds to some packages. I didn't accept these changes as-is because they are unnecessarily strict (elapid has been tested and works with sklearn 1.1.2, with geopandas 0.11.1, etc.).

saraedum · 2022-09-28T16:26:26Z

recipes/elapid/meta.yaml

+    - python
+    - descartes >=1.1.0
+    - geopandas >=0.7.0
+    - glmnet >=2.0,<2.3  # [not win] windows build unavailable; elapid will use sklearn features instead


As a suggestion for the next release of elapid, you could consider just issuing a warning if glmnet is not available, remove this dependency and set this recipe to noarch: python. The upside is that it will work without any further changes on apple silicon, aarch64, ….

I've removed the glmnet requirement in the recipe and set it to noarch: python. It already has a try/except for importing glmnet so it should be fine as-is.

saraedum · 2022-09-28T16:27:09Z

Sorry for the delayed reaction here. This looks alright to me. Let me know if you want to make any further changes.

conda-forge-linter · 2022-10-03T04:40:59Z

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipes/elapid) and found some lint.

Here's what I've got...

For recipes/elapid:

noarch: python recipes are required to have a lower bound on the python version. Typically this means putting python >=3.6 in both host and run but you should check upstream for the package's Python compatibility.

conda-forge-linter · 2022-10-03T04:46:32Z

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipes/elapid) and found some lint.

Here's what I've got...

For recipes/elapid:

requirements: host: python>=3.7 must contain a space between the name and the pin, i.e. python >=3.7
requirements: run: python>=3.7 must contain a space between the name and the pin, i.e. python >=3.7
noarch: python recipes are required to have a lower bound on the python version. Typically this means putting python >=3.6 in both host and run but you should check upstream for the package's Python compatibility.

conda-forge-linter · 2022-10-03T04:48:29Z

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/elapid) and found it was in an excellent condition.

earth-chris · 2022-10-03T05:12:47Z

alright @saraedum I think I've addressed your requested changes.

added elapid meta.yaml

013a191

skipping windows build

bb62b1e

removing noarch to explicitly set windows exclusion

427a2a7

removing python version to comply with os-dependent builds

98899a6

update to 3.12

1cd0a12

github-actions bot added review-requested staged-recipes labels Sep 15, 2022

incrementing to 0.3.13 while waiting

6f5e7e4

saraedum reviewed Sep 19, 2022

View reviewed changes

recipes/elapid/meta.yaml Outdated Show resolved Hide resolved

saraedum reviewed Sep 19, 2022

View reviewed changes

recipes/elapid/meta.yaml Show resolved Hide resolved

saraedum reviewed Sep 19, 2022

View reviewed changes

recipes/elapid/meta.yaml Show resolved Hide resolved

saraedum requested changes Sep 19, 2022

View reviewed changes

Christopher Anderson and others added 2 commits September 19, 2022 17:05

Apply suggestions from code review

bb442a8

removed <py37 restriction, added dev_url Co-authored-by: Julian Rüth <[email protected]>

add context for no windows build for glmnet

806b2b8

earth-chris requested a review from saraedum September 20, 2022 20:28

earth-chris added 2 commits September 23, 2022 09:18

increment to 0.3.14 while waiting

92c8270

pinned upper bound of glmnet version

de57b3f

saraedum reviewed Sep 28, 2022

View reviewed changes

saraedum removed the review-requested label Sep 28, 2022

saraedum added the Awaiting author contribution label Sep 28, 2022

restricting upper versions

5b55ea6

adding minimum python version

b73d350

properly fomatting version spacing

e361b15

saraedum merged commit 93b91fa into conda-forge:main Oct 3, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding elapid #18896

Adding elapid #18896

earth-chris commented May 10, 2022

conda-forge-linter commented May 10, 2022

conda-forge-linter commented May 11, 2022

conda-forge-linter commented May 11, 2022

conda-forge-linter commented May 11, 2022

earth-chris commented Sep 15, 2022

conda-forge-linter commented Sep 15, 2022

saraedum Sep 19, 2022

earth-chris Sep 19, 2022

amirhessam88 Sep 22, 2022

earth-chris Sep 22, 2022

saraedum left a comment

earth-chris commented Sep 20, 2022

earth-chris commented Sep 27, 2022

conda-forge-linter commented Sep 27, 2022

saraedum Sep 28, 2022

earth-chris Oct 3, 2022

saraedum Sep 28, 2022

earth-chris Oct 3, 2022

saraedum commented Sep 28, 2022

conda-forge-linter commented Oct 3, 2022

conda-forge-linter commented Oct 3, 2022

conda-forge-linter commented Oct 3, 2022

earth-chris commented Oct 3, 2022

Adding elapid #18896

Adding elapid #18896

Conversation

earth-chris commented May 10, 2022

conda-forge-linter commented May 10, 2022

conda-forge-linter commented May 11, 2022

conda-forge-linter commented May 11, 2022

conda-forge-linter commented May 11, 2022

earth-chris commented Sep 15, 2022

conda-forge-linter commented Sep 15, 2022

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

saraedum left a comment

Choose a reason for hiding this comment

earth-chris commented Sep 20, 2022

earth-chris commented Sep 27, 2022

conda-forge-linter commented Sep 27, 2022

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

saraedum commented Sep 28, 2022

conda-forge-linter commented Oct 3, 2022

conda-forge-linter commented Oct 3, 2022

conda-forge-linter commented Oct 3, 2022

earth-chris commented Oct 3, 2022