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

feat: it's a bird, it's a plane, it's vortex-all! #1140

Merged
merged 20 commits into from
Nov 4, 2024
Merged

Conversation

danking
Copy link
Member

@danking danking commented Oct 25, 2024

One crate to rule them all and in the darkness bind them.

One crate to rule them all and in the darkness bind them.
@danking danking marked this pull request as ready for review October 25, 2024 15:41
@danking danking enabled auto-merge (squash) October 25, 2024 15:41
@@ -0,0 +1,42 @@
[package]
name = "vortex-rs"
description = "Vortex with all builtin codecs and a sampling compressor."
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want this crate to re-export all the underlying dependencies?

Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably also move a bunch of the docs in vortex-array here, but that can be a separate PR

@robert3005
Copy link
Member

@danking as @AdamGS points out you have to reexport dependencies, otherwise none of the code referenced is accessible and it's just a dead dependency

@AdamGS
Copy link
Contributor

AdamGS commented Oct 25, 2024

and probably with some module structure, the same way arrow has arrow::schema and the arrow_schema crate

@danking
Copy link
Member Author

danking commented Oct 25, 2024

I did this:

pub use vortex::*;
pub use {
    vortex_alp as alp, vortex_buffer as buffer, vortex_bytebool as bytebool,
    vortex_datetime_dtype as datetime_dtype, vortex_datetime_parts as datetime_parts,
    vortex_dict as dict, vortex_dtype as dtype, vortex_error as error, vortex_expr as expr,
    vortex_fastlanes as fastlanes, vortex_flatbuffers as flatbuffers, vortex_fsst as fsst,
    vortex_proto as proto, vortex_roaring as roaring, vortex_runend as runend,
    vortex_runend_bool as runend_bool, vortex_sampling_compressor as sampling_compressor,
    vortex_scalar as scalar, vortex_schema as schema, vortex_serde as serde,
    vortex_zigzag as zigzag,
};

@danking
Copy link
Member Author

danking commented Oct 25, 2024

You can see what it looks like in bench-vortex and pyvortex now.

@danking
Copy link
Member Author

danking commented Oct 25, 2024

I think we liked vortex as the module name and vortex-all as the package

@danking danking disabled auto-merge October 28, 2024 21:19
@danking
Copy link
Member Author

danking commented Oct 28, 2024

Auto merge disabled until #1124 merges.

@danking danking changed the title feat: super-crate vortex-rs feat: it's a bird, it's a plane, it's vortex-all! Oct 29, 2024
@danking
Copy link
Member Author

danking commented Nov 1, 2024

@robert3005 Do you have more mega PRs to land?

@robert3005
Copy link
Member

None that you need to block on

@danking danking enabled auto-merge (squash) November 4, 2024 15:32
@lwwmanning lwwmanning disabled auto-merge November 4, 2024 19:27
@lwwmanning lwwmanning enabled auto-merge (squash) November 4, 2024 19:27
@lwwmanning lwwmanning merged commit 95fbe05 into develop Nov 4, 2024
12 checks passed
@lwwmanning lwwmanning deleted the dk/vortex-rs branch November 4, 2024 19:45
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.

4 participants