Skip to content

Conversation

lgray
Copy link

@lgray lgray commented Jul 16, 2025

This is just a suggestion, happy to discuss, but thinking to the long time based on some things we learned in dask in high energy physics. Specifically that our graphs can get very large and a compiled taskgraph api may be something we desire, likewise being able to easily include extensions may help the project in the future. I know this is a premature optimization, but it also doesn't negatively affect anything.

For now, running cmake in scikit-build-core is turned off since this repository has no compiled code.
If the project goes with rust, maturin is an easy change from here.

I have removed the setup.py and setup.cfg, so all configuration is in pyproject.toml and changed the build system to hatchling.
I have also moved cubed's implementation into a src/ directory so there's no possibility for clobbering packages.

Tests may fail for a bit here where I find all the rough edges from the change.

Curious to hear your thoughts!

@tomwhite
Copy link
Member

Thanks for the PR @lgray - a few comments below.

Specifically that our graphs can get very large and a compiled taskgraph api may be something we desire

Cubed should be able to help with this since each node in the graph is an array not a chunk.

For now, running cmake in scikit-build-core is turned off since this repository has no compiled code.

I would rather make this change when it is needed - i.e. alongside the PR that introduces the compiled code.

If the project goes with rust, maturin is an easy change from here.

I can see some parts of the Cubed stack migrating to Rust in the future. For storage we already have obstore support (#715), and zarrs-python (#731) is on the TODO list. Both of these are simply using Rust-based Python libraries though so don't need any build changes, but we could have Rust code in Cubed for the runtime, primitive (blockwise, rechunk) and core (planning) layers. I think it's natural to make the necessary build changes at the point we introduce the compiled code.

I have removed the setup.py and setup.cfg, so all configuration is in pyproject.toml.

This is a good change - could you break this off as a separate PR and we'll get it merged.

@lgray
Copy link
Author

lgray commented Jul 17, 2025

OK - I'll move everything to hatchling in the meantime. Do you have any opinion on src/ vs the package in the root of the repository?

@tomwhite
Copy link
Member

OK - I'll move everything to hatchling in the meantime. Do you have any opinion on src/ vs the package in the root of the repository?

I don't have a strong opinion on that. @TomNicholas do you have any thoughts?

@TomNicholas
Copy link
Member

No strong opinion on that either

@lgray
Copy link
Author

lgray commented Jul 17, 2025

Gotcha - I only did it here because not doing so has gotten me into trouble with consistent testing a few times in the past.

@lgray lgray force-pushed the use-scikit-build-core branch from 05b31e5 to 8bdf062 Compare July 17, 2025 16:24
@lgray lgray changed the title build: use scikit-build-core build: use hatchling Jul 17, 2025
@lgray
Copy link
Author

lgray commented Jul 17, 2025

OK - kept the pyproject.toml-only changes, used a python-only / more minimal build system. All the code is now beneath src/

@tomwhite
Copy link
Member

I just enabled the CI - many are passing! But looks like .coveragerc is not being picked up?

@lgray
Copy link
Author

lgray commented Jul 17, 2025

Thanks! I'll see what I can do to get it all going. It seems to not be picking up the mypy config correctly either.

@tomwhite
Copy link
Member

Thanks. Don't worry about array API, lithops and Zarr v2 - they are known failures on CI at least (#752, #753).

@lgray
Copy link
Author

lgray commented Jul 20, 2025

Hey - sorry for dropping off here. I did start a vacation last week, given this is fairly low priority I'll tie it up on my return.

Thank you very much for the info about known failing tests so I don't pull my hair out. :-)

@tomwhite
Copy link
Member

No problem @lgray! Moving the src dir may require a bit of coordination with other PRs so let me know when you're ready.

I've also been fixing some of the flaky test failures.

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