Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add python bindings #33

Closed
wants to merge 44 commits into from
Closed

add python bindings #33

wants to merge 44 commits into from

Conversation

isuruf
Copy link
Member

@isuruf isuruf commented Feb 6, 2023

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

This allows for constructing MLIR using Python classes rather than C++ classes.
Programs can be written, compiled, and executed all from within Python.
I don't like this solution, but it is better to get something
working and kick this problem down the road. Mac M1 machines
can always build an x86-64 environment, so this is more of an
inconvenience than a show-stopper for any user.
This allows for constructing MLIR using Python classes rather than C++ classes.
Programs can be written, compiled, and executed all from within Python.
I don't like this solution, but it is better to get something
working and kick this problem down the road. Mac M1 machines
can always build an x86-64 environment, so this is more of an
inconvenience than a show-stopper for any user.
@conda-forge-webservices
Copy link
Contributor

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

@isuruf isuruf marked this pull request as draft February 6, 2023 18:12
@isuruf
Copy link
Member Author

isuruf commented Feb 7, 2023

cc @jim22k

@jim22k
Copy link

jim22k commented Feb 7, 2023

@isuruf Thanks for picking this back up! I wasn't sure if it could work with only one output depending on python+numpy.

I started to add a separate mlir-python-bindings feedstock pinned to the specific MLIR version, but if we can get this working within this feedstock, it will be less of a maintenance burden for new releases of MLIR.

Comment on lines +11 to +13
# Can't have llvmdev in BUILD_PREFIX as llvmdev conflicts with the clang compiler
# TODO: remove conflict between llvmdev and clang compiler
conda create -p $BUILD_PREFIX/llvm llvmdev=$PKG_VERSION -c conda-forge --yes --quiet
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a superset of conda-forge/clangdev-feedstock#196, where we can't mix our OSX compilers with newer libclang, or in this case: llvm.

Would this be solvable by using clang_bootstrap here? Otherwise I can't see how we can build clang against a shared libllvm (though currently we don't distinguish that precisely, we just have llvmdev == {{ version }} in host/run everywhere; so perhaps this can be broken down to be more finer-grained, or we build clang against a static llvm)

@jim22k
Copy link

jim22k commented Feb 8, 2023

Is this only an issue with OSX Arm? If so, I would be fine with excluding that case. Users who need the mlir-python-bindings on M1 Mac can still create an x86 conda environment.

@jim22k
Copy link

jim22k commented Feb 13, 2023

@isuruf My attempt at creating a separate mlir-python-bindings feedstock appears to be building correctly. Do you want to keep trying make it work on the mlir feedstock or should I push for the separate feedstock? I'm assuming we should only go with one of the options to avoid a name conflict.

@h-vetinari
Copy link
Member

Closing this as solved by conda-forge/staged-recipes#21954

@h-vetinari h-vetinari closed this Apr 7, 2023
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.

3 participants