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

Implement Python bindings #10

Merged
merged 15 commits into from
Oct 10, 2024
Merged

Implement Python bindings #10

merged 15 commits into from
Oct 10, 2024

Conversation

aliddell
Copy link
Member

@aliddell aliddell commented Oct 9, 2024

  • Implement Python bindings
    • Add a CMake option (off by default) to build Python bindings
    • Add setup.py, pyproject.toml, and MANIFEST.in with which you can install acquire_zarr in your environment
    • Add a job to build Python wheels

Tests coming later. Stay tuned...

@aliddell aliddell requested a review from jeskesen October 9, 2024 17:24
Base automatically changed from python-bindings-prep to main October 9, 2024 20:16
@jeskesen
Copy link
Contributor

LGTM, but I'd like to get someone else's opinion on the Python packaging, because I don't know that part.

Copy link
Contributor

@jeskesen jeskesen left a comment

Choose a reason for hiding this comment

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

So, from my perspective, it wasn't readily understandable how to build the new python bindings. I did it the "CMake way", with a configure/build/install step, and they weren't installed in the "install" directory (as it appears there's no install step added in the

Over Slack, @aliddell showed me how to build it the "python" way: python -m build, and that worked fine. So, I'll approve this, but can we open a new ticket to update build instructions?

.value("V3", ZarrVersion_3)
.export_values();

py::enum_<ZarrDataType>(m, "DType")
Copy link
Contributor

Choose a reason for hiding this comment

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

A topic for discussion: do we think it might be worth building ways to implicitly (or explicitly) convert between numpy's DType & ZarrDataTypes?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I think that might be worthwhile to do.

@aliddell aliddell merged commit c1cb57b into main Oct 10, 2024
13 checks passed
@aliddell aliddell deleted the python-bindings branch October 10, 2024 18:55
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.

2 participants