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

remove cython dependency, remove setup.cfg #10

Merged
merged 1 commit into from
Dec 30, 2024

Conversation

jameslamb
Copy link
Member

Came here looking to remove the setup.cfg file, inspired by rapidsai/raft#2532.

But this PR uncovered a few other packaging things to be cleaned up. Proposes:

  • removing Cython dependency
  • aligning cmake_minimum_required() version with the one from the conda recipe, and with what RAPIDS has been using for the last few releases
  • removing maptlolib dependency for benchmarks

Notes for Reviewers

How is it safe to remove Cython?

As far as I can tell, this project doesn't have any Cython code. It is building a shared library and then loading it with ctypes:

def dlopen_no_autoclose(ffi: Any, lib_path: str) -> Any:
# Use an already-opened library handle, which cffi will convert to a
# regular FFI object (using the definitions previously added using
# ffi.cdef), but will not automatically dlclose() on collection.
lib = CDLL(lib_path, mode=RTLD_GLOBAL)
return ffi.dlopen(ffi.cast("void *", lib._handle))

@jameslamb jameslamb added improvement Improves an existing functionality non-breaking Introduces a non-breaking change labels Dec 23, 2024
@jameslamb jameslamb changed the title WIP: remove cython dependency, remove setup.cfg remove cython dependency, remove setup.cfg Dec 23, 2024
@jameslamb jameslamb requested a review from seberg December 23, 2024 18:40
@jameslamb jameslamb marked this pull request as ready for review December 23, 2024 18:41
Copy link
Contributor

@seberg seberg left a comment

Choose a reason for hiding this comment

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

Great cleanups, thanks! Adding black also is good.

@@ -20,7 +20,6 @@ dependencies:
- cupy>=12.0.0
- cupynumeric==25.01.*,>=0.0.0.dev0
- cxx-compiler
- cython>=3.0.3
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, nice thanks... That must have come from legate-df.

@seberg seberg merged commit d69399e into rapidsai:main Dec 30, 2024
8 checks passed
@jameslamb jameslamb deleted the packaging-cleanup branch December 30, 2024 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improves an existing functionality non-breaking Introduces a non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants