Add polars #18906

Maxyme · 2022-05-11T19:30:53Z

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-11T19:31:12Z

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/polars) and found some lint.

Here's what I've got...

For recipes/polars:

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
requirements: run: numpy>=1.16.0 must contain a space between the name and the pin, i.e. numpy >=1.16.0
requirements: run: typing_extensions>=4.0.0 must contain a space between the name and the pin, i.e. typing_extensions >=4.0.0

conda-forge-linter · 2022-05-11T19:33:35Z

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/polars) and found some lint.

Here's what I've got...

For recipes/polars:

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-11T20:15:47Z

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/polars) and found it was in an excellent condition.

dhirschfeld · 2022-05-12T21:32:47Z

Super excited to see this! It seems like it's a very awkward one to compile for conda-forge though 😬.

If Windows is being particularly difficult perhaps this can be merged without Windows support and then debug it on the feedstock?

conda-forge-linter · 2022-05-12T22:30:48Z

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/polars) and found some lint.

Here's what I've got...

For recipes/polars:

There are too few lines. There should be one empty line at the end of the file.

conda-forge-linter · 2022-05-12T22:31:52Z

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/polars) and found it was in an excellent condition.

Maxyme · 2022-05-12T22:32:10Z

@dhirschfeld, just updated to skip windows builds for now, I think it will be quite simple to fix in a future PR. Thanks for the comment.

dhirschfeld · 2022-05-12T23:05:47Z

Seems like it can't find the python executable, which is odd since you explicitly specify --interpreter=%PYTHON% to the maturin command.

--- stderr 
error: failed to run the Python interpreter at C:\bld\polars_1652374306242\_build_env\python.exe: The system cannot find the file specified. (os error 2) 
warning: build failed, waiting for other jobs to finish... 
💥 maturin failed 
Caused by: Failed to build a native library through cargo 
Caused by: Cargo build finished with "exit code: 101": `cargo rustc --manifest-path Cargo.toml --message-format json --release --lib -- -C codegen-units=16 -C lto=thin -C target-cpu=native -C link-arg=-s`

Maybe the selector means python isn't being installed in the build env? 🤔

      - python                              # [build_platform != target_platform]

Anyway, easy enough to test that theory out in the feedstock....

ritchie46

Awesome initiative! I've left a few comments regarding how we build the binary.

ritchie46 · 2022-05-13T06:49:45Z

recipes/polars/build.sh

+
+export PATH=${SRC_DIR}/rust-nightly-install/bin:$PATH
+
+maturin build --no-sdist --release --strip --manylinux off --interpreter="${PYTHON}" --rustc-extra-args="-C codegen-units=16 -C lto=thin -C target-cpu=native"


We should not be compiling for native target CPU. That would mean the binary is unusable for CPU's that don't share the same features.

For pypi we set:

export RUSTFLAGS='-C target-feature=+fxsr,+sse,+sse2,+sse3,+ssse3,+sse4.1,+sse4.2,+popcnt,+avx,+fma'

This is a reasonable default.

Next, we also should not set the codegen-units nor thin lto. This creates a binary that would be slower than what we produce for pypi.

So I would propose skipping the whole --rustc-extra-args="-C codegen-units=16 -C lto=thin -C target-cpu=native" and setting:

export RUSTFLAGS='-C target-feature=+fxsr,+sse,+sse2,+sse3,+ssse3,+sse4.1,+sse4.2,+popcnt,+avx,+fma'

we may have to modify this once the feedstock is created for mac arm builds since both the x86 and arm builds will use build.sh

We don't enable avx by default on conda-forge binaries. So, please remove RUSTFLAGS.

…ch pypi binary

timkpaine · 2022-05-13T14:37:25Z

windows looks ok over here:Maxyme#1
will ping again once the builds are confirmed passing

sugatoray · 2022-05-14T07:08:21Z

@BastianZim @beckermr @bollwyvl Requesting your feedback on the recipe.

The sources for rust-nightly are included in the recipe. Does it require the license(s) to be included for rust-nightly as well? Thank you.

xhochy

Can we instead please update the rust_dev build of the rust conpiler feedstock instead of bundling it here?

timkpaine · 2022-05-15T13:16:57Z

@xhochy is there an existing pattern for using the rust_dev build like this? Just keeping in mind that this is the 5th attempt at getting a polars feedstock in and there's a dearth of rust-nightly examples to work from

xhochy · 2022-05-15T18:38:59Z

@xhochy is there an existing pattern for using the rust_dev build like this?

Add the following to conda_build_config.yaml:

channel_sources:
 - conda-forge/label/rust_dev,conda-forge

You might need to update the rust_dev build in the rust-feedstock to get the latest version there.

Just keeping in mind that this is the 5th attempt at getting a polars feedstock in and there's a dearth of rust-nightly examples to work from

The number of failed PRs should not be a wildcard to dismiss reviews. We want sustainable solutions here.

isuruf · 2022-05-18T18:55:12Z

recipes/polars/build.sh

+# todo: Check +avx when supported in conda-forge
+export RUSTFLAGS='-C target-feature=+fxsr,+sse,+sse2,+sse3,+ssse3,+sse4.1,+sse4.2,+popcnt,+fma'


Remove this so that the default options are used and when conda-forge moves forward, it'll be done automatically.

Note that we use these feature flags in polars. Or are they moved somewhere else? I am not really familiar with conda feedstocks.

@ritchie46 In theory - if I'm correct... and if it compiles, it will use library calls instead of cpu instructions.
I think that we can either compile with these flags and force cpu instruction optimizations for some features and potentially lock out some users, or use default flags, (I don't think conda sets RUSTFLAGS but I could be wrong) and lose some performance at the benefit of allowing more users to use the library (see: https://internals.rust-lang.org/t/policies-around-default-cpu-architecture/9142/5).
Also according to this (https://store.steampowered.com/hwsurvey -> other settings) avx would be supported by 95% of users.

Yes thats correct but these are reasonable defaults that practically all machines have. The rust defaults are super conservative because they also ship to embedded.

Note that we don't activate avx. That one is not yet broadly supported.

Would you be okay going ahead with the defaults and looking at the flags in a separate pr? I can open a new one after this for that purpose. @isuruf ?

timkpaine · 2022-05-19T18:53:48Z

ive been duplicating our work in this PR on #18987 so whenever we merge this one if we could do connector-x as well (same deal, rust nightly).

Maxyme · 2022-05-21T15:30:29Z

Small update: this PR is currently blocked by: #19036

jakirkham · 2022-06-03T03:56:29Z

Now that PR ( #19036 ) is in would it be possible to drop those changes from here?

xhochy · 2022-06-03T06:55:00Z

@Maxyme Can you rebase and remove all changes that aren't inside recipe/polars? Then we can finally merge :)

Also I'm happy to be added as a maintainer to help with future build troubles if you would like to.

dhirschfeld · 2022-06-03T08:47:02Z

Thanks @xhochy for getting this over the line, and making it easier to add new rust recipes! 🎉

conda-forge-linter · 2022-06-03T11:09:45Z

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/polars) and found some lint.

Here's what I've got...

For recipes/polars:

There are too few lines. There should be one empty line at the end of the file.

conda-forge-linter · 2022-06-03T11:13:13Z

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/polars) and found it was in an excellent condition.

dhirschfeld · 2022-06-03T11:19:47Z

recipes/polars/meta.yaml

+    - ritchie46
+    - sugatoray
+    - xhochy
+    - dhirschfeld


☝️ I'm happy to be added as a maintainer

Maxyme · 2022-06-03T18:04:25Z

@isuruf, ready to merge this!

timkpaine · 2022-06-03T18:05:22Z

(applying the same changes to #18987)

timkpaine · 2022-06-03T18:28:02Z

moving to #19136

timkpaine · 2022-06-07T15:54:13Z

🎉 🎉 🎉

Add polars recipe

33377ae

Update packages

2713cb7

Maxyme force-pushed the add_polars branch from 6fc50e9 to 2713cb7 Compare May 11, 2022 19:32

Fix linting

be9e26e

Rename build.bat -> bld.bat and add cmake

2773464

Maxyme mentioned this pull request May 11, 2022

Provide Anaconda package pola-rs/polars#383

Closed

Maxyme added 3 commits May 12, 2022 09:31

Add make for all platforms

8c5334e

Set CMAKE generator for windows

f1c31ad

Move set generator before maturin build

3413895

timkpaine mentioned this pull request May 12, 2022

Add polars to conda-forge, another attempt #18244

Closed

9 tasks

Add early exits in bld.bat, set c compiler to vs2019 for win

b204476

Skip windows builds, add other maintainers for recipe

3c650a7

Add empty line

c83dca9

Maxyme marked this pull request as ready for review May 13, 2022 01:43

timkpaine mentioned this pull request May 13, 2022

Adding polars to conda-forge #16717

Closed

9 tasks

ritchie46 reviewed May 13, 2022

View reviewed changes

Remove --rustc-extra-args in maturin and instead set rustflags to mat…

c676459

…ch pypi binary

xhochy requested changes May 15, 2022

View reviewed changes

isuruf reviewed May 18, 2022

View reviewed changes

Maxyme added 2 commits May 18, 2022 15:54

Remove all compilation flag and update to 0.13.37

e81ff8d

Update SHA256

ec82600

dhirschfeld mentioned this pull request May 19, 2022

vegafusion-python-embed v0.4.0 conda-forge/vegafusion-python-embed-feedstock#6

Closed

3 tasks

Maxyme requested a review from xhochy May 19, 2022 16:52

timkpaine mentioned this pull request May 19, 2022

Switch to rust conda package conda-forge/orjson-feedstock#33

Closed

xhochy mentioned this pull request May 21, 2022

Support using channel_urls from conda_build_config.yaml #19036

Merged

Merge branch 'channel-urls' into add_polars

4e2d88d

xhochy force-pushed the add_polars branch from aa2e368 to 4e2d88d Compare June 2, 2022 18:15

Maxyme and others added 2 commits June 3, 2022 06:41

Merge branch 'conda-forge:main' into add_polars

64e57f8

Add xhochy and dhirschfeld as maintainers

caf5e9b

Merge .ci_support from master

009dc1e

Maxyme force-pushed the add_polars branch from d92fa94 to 009dc1e Compare June 3, 2022 11:12

dhirschfeld reviewed Jun 3, 2022

View reviewed changes

xhochy approved these changes Jun 3, 2022

View reviewed changes

dhirschfeld approved these changes Jun 6, 2022

View reviewed changes

xhochy merged commit ec7c7dd into conda-forge:main Jun 7, 2022

Maxyme deleted the add_polars branch June 9, 2022 13:59

timkpaine mentioned this pull request Aug 19, 2022

polars v0.14.2 conda-forge/polars-feedstock#37

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add polars #18906

Add polars #18906

Maxyme commented May 11, 2022 •

edited

Loading

conda-forge-linter commented May 11, 2022

conda-forge-linter commented May 11, 2022

conda-forge-linter commented May 11, 2022

dhirschfeld commented May 12, 2022

conda-forge-linter commented May 12, 2022

conda-forge-linter commented May 12, 2022

Maxyme commented May 12, 2022

dhirschfeld commented May 12, 2022

ritchie46 left a comment

ritchie46 May 13, 2022

timkpaine May 13, 2022 •

edited

Loading

isuruf May 17, 2022

timkpaine commented May 13, 2022

sugatoray commented May 14, 2022

xhochy left a comment

timkpaine commented May 15, 2022 •

edited

Loading

xhochy commented May 15, 2022

isuruf May 18, 2022

ritchie46 May 18, 2022

Maxyme May 18, 2022 •

edited

Loading

ritchie46 May 19, 2022

Maxyme May 19, 2022

timkpaine commented May 19, 2022

Maxyme commented May 21, 2022

jakirkham commented Jun 3, 2022

xhochy commented Jun 3, 2022

dhirschfeld commented Jun 3, 2022

conda-forge-linter commented Jun 3, 2022

conda-forge-linter commented Jun 3, 2022

dhirschfeld Jun 3, 2022

Maxyme commented Jun 3, 2022

timkpaine commented Jun 3, 2022

timkpaine commented Jun 3, 2022

timkpaine commented Jun 7, 2022


		export PATH=${SRC_DIR}/rust-nightly-install/bin:$PATH

		maturin build --no-sdist --release --strip --manylinux off --interpreter="${PYTHON}" --rustc-extra-args="-C codegen-units=16 -C lto=thin -C target-cpu=native"

		# todo: Check +avx when supported in conda-forge
		export RUSTFLAGS='-C target-feature=+fxsr,+sse,+sse2,+sse3,+ssse3,+sse4.1,+sse4.2,+popcnt,+fma'

Add polars #18906

Add polars #18906

Conversation

Maxyme commented May 11, 2022 • edited Loading

conda-forge-linter commented May 11, 2022

conda-forge-linter commented May 11, 2022

conda-forge-linter commented May 11, 2022

dhirschfeld commented May 12, 2022

conda-forge-linter commented May 12, 2022

conda-forge-linter commented May 12, 2022

Maxyme commented May 12, 2022

dhirschfeld commented May 12, 2022

ritchie46 left a comment

Choose a reason for hiding this comment

ritchie46 May 13, 2022

Choose a reason for hiding this comment

timkpaine May 13, 2022 • edited Loading

Choose a reason for hiding this comment

isuruf May 17, 2022

Choose a reason for hiding this comment

timkpaine commented May 13, 2022

sugatoray commented May 14, 2022

xhochy left a comment

Choose a reason for hiding this comment

timkpaine commented May 15, 2022 • edited Loading

xhochy commented May 15, 2022

isuruf May 18, 2022

Choose a reason for hiding this comment

ritchie46 May 18, 2022

Choose a reason for hiding this comment

Maxyme May 18, 2022 • edited Loading

Choose a reason for hiding this comment

ritchie46 May 19, 2022

Choose a reason for hiding this comment

Maxyme May 19, 2022

Choose a reason for hiding this comment

timkpaine commented May 19, 2022

Maxyme commented May 21, 2022

jakirkham commented Jun 3, 2022

xhochy commented Jun 3, 2022

dhirschfeld commented Jun 3, 2022

conda-forge-linter commented Jun 3, 2022

conda-forge-linter commented Jun 3, 2022

dhirschfeld Jun 3, 2022

Choose a reason for hiding this comment

Maxyme commented Jun 3, 2022

timkpaine commented Jun 3, 2022

timkpaine commented Jun 3, 2022

timkpaine commented Jun 7, 2022

Maxyme commented May 11, 2022 •

edited

Loading

timkpaine May 13, 2022 •

edited

Loading

timkpaine commented May 15, 2022 •

edited

Loading

Maxyme May 18, 2022 •

edited

Loading