refactor(streaming): flatten namespaces and rename to snake_case - #22909
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR flattens the Changescudf_streaming API flattening and callsite migration
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cpp/libcudf_streaming/include/cudf_streaming/utils.hpp (1)
15-73:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftCRITICAL: Public API namespace move lacks a compatibility/deprecation bridge
Moving exported
str(...)andestimated_memory_usage(...)fromcudf_streaming::integrationstocudf_streamingis a hard compile-time break for downstream consumers, and this header does not provide deprecated forwarding aliases.Suggested compatibility bridge (transitional)
namespace cudf_streaming { // new declarations... } // namespace cudf_streaming +namespace cudf_streaming::integrations { +[[deprecated("Use cudf_streaming::str")]] +inline std::string str(cudf::column_view col, + cudf::size_type index, + rmm::cuda_stream_view stream, + rmm::device_async_resource_ref mr) +{ + return cudf_streaming::str(col, index, stream, mr); +} + +[[deprecated("Use cudf_streaming::str")]] +inline std::string str(cudf::column_view col, + rmm::cuda_stream_view stream, + rmm::device_async_resource_ref mr) +{ + return cudf_streaming::str(col, stream, mr); +} + +[[deprecated("Use cudf_streaming::str")]] +inline std::string str(cudf::table_view tbl, + rmm::cuda_stream_view stream, + rmm::device_async_resource_ref mr) +{ + return cudf_streaming::str(tbl, stream, mr); +} + +[[deprecated("Use cudf_streaming::estimated_memory_usage")]] +inline std::size_t estimated_memory_usage(cudf::column_view const& col, + rmm::cuda_stream_view stream) +{ + return cudf_streaming::estimated_memory_usage(col, stream); +} + +[[deprecated("Use cudf_streaming::estimated_memory_usage")]] +inline std::size_t estimated_memory_usage(cudf::table_view const& tbl, + rmm::cuda_stream_view stream) +{ + return cudf_streaming::estimated_memory_usage(tbl, stream); +} +} // namespace cudf_streaming::integrationsAs per coding guidelines, “C++ API changes without proper deprecation warnings” are CRITICAL; PR objectives state this refactor is a hard break without aliases.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cpp/libcudf_streaming/include/cudf_streaming/utils.hpp` around lines 15 - 73, The functions str(...) with two overloads (one taking an index parameter and one without) and estimated_memory_usage(...) with two overloads have been moved from the cudf_streaming::integrations namespace to the cudf_streaming namespace, breaking backward compatibility for downstream consumers. Add deprecated forwarding aliases in the cudf_streaming::integrations namespace that delegate to the new cudf_streaming implementations. Each forwarding function should be marked with appropriate deprecation warnings to guide users to migrate to the new namespace location while maintaining compile-time compatibility during the transition period.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cpp/libcudf_streaming/include/cudf_streaming/table_chunk.hpp`:
- Around line 38-57: The `exclusive_view` enum and other public symbols in the
`table_chunk` class have been renamed without providing backward-compatible
aliases, creating a hard breaking change for downstream users. For each renamed
public symbol across all affected locations (the enum class definition, and the
additional sites at lines 93-97, 172-173, 187-188, 209-213, 264-265, 325-326,
and 334-335), add deprecated aliases or type definitions that preserve the old
names and map them to the new ones. Include deprecation attributes (such as
[[deprecated]]) with clear migration notes directing users to the new names,
allowing one release cycle for downstream code to transition before the old
names are fully removed.
In `@python/cudf_streaming/cudf_streaming/__init__.py`:
- Around line 15-56: The __init__.py file is now fully flattened and completely
removes the old module paths (cudf_streaming.streaming.* and
cudf_streaming.integrations.*) without a deprecation period, which breaks
backward compatibility. Add a deprecation compatibility layer that intercepts
imports from the old paths and either redirects them to the new locations with a
deprecation warning or provides deprecated module aliases. Use Python's
module-level __getattr__ function or sys.modules manipulation to handle attempts
to import from the legacy cudf_streaming.streaming and
cudf_streaming.integrations modules, ensuring users receive clear deprecation
warnings that guide them to use the new flattened import paths instead of
causing immediate ImportError failures.
---
Outside diff comments:
In `@cpp/libcudf_streaming/include/cudf_streaming/utils.hpp`:
- Around line 15-73: The functions str(...) with two overloads (one taking an
index parameter and one without) and estimated_memory_usage(...) with two
overloads have been moved from the cudf_streaming::integrations namespace to the
cudf_streaming namespace, breaking backward compatibility for downstream
consumers. Add deprecated forwarding aliases in the cudf_streaming::integrations
namespace that delegate to the new cudf_streaming implementations. Each
forwarding function should be marked with appropriate deprecation warnings to
guide users to migrate to the new namespace location while maintaining
compile-time compatibility during the transition period.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 9f02d121-b0b3-4ab1-84b5-dd2fb0f17979
📒 Files selected for processing (117)
cpp/libcudf_streaming/CMakeLists.txtcpp/libcudf_streaming/benchmarks/bench_partition.cppcpp/libcudf_streaming/benchmarks/bench_shuffle.cppcpp/libcudf_streaming/benchmarks/streaming/bench_streaming_shuffle.cppcpp/libcudf_streaming/benchmarks/streaming/data_generator.hppcpp/libcudf_streaming/benchmarks/streaming/ndsh/bench_read.cppcpp/libcudf_streaming/benchmarks/streaming/ndsh/concatenate.cppcpp/libcudf_streaming/benchmarks/streaming/ndsh/concatenate.hppcpp/libcudf_streaming/benchmarks/streaming/ndsh/groupby.cppcpp/libcudf_streaming/benchmarks/streaming/ndsh/groupby.hppcpp/libcudf_streaming/benchmarks/streaming/ndsh/join.cppcpp/libcudf_streaming/benchmarks/streaming/ndsh/join.hppcpp/libcudf_streaming/benchmarks/streaming/ndsh/parquet_writer.cppcpp/libcudf_streaming/benchmarks/streaming/ndsh/parquet_writer.hppcpp/libcudf_streaming/benchmarks/streaming/ndsh/q01.cppcpp/libcudf_streaming/benchmarks/streaming/ndsh/q03.cppcpp/libcudf_streaming/benchmarks/streaming/ndsh/q04.cppcpp/libcudf_streaming/benchmarks/streaming/ndsh/q09.cppcpp/libcudf_streaming/benchmarks/streaming/ndsh/q21.cppcpp/libcudf_streaming/benchmarks/streaming/ndsh/sort.cppcpp/libcudf_streaming/benchmarks/streaming/ndsh/sort.hppcpp/libcudf_streaming/benchmarks/streaming/ndsh/utils.cppcpp/libcudf_streaming/benchmarks/streaming/ndsh/utils.hppcpp/libcudf_streaming/examples/example_shuffle.cppcpp/libcudf_streaming/include/cudf_streaming/bloom_filter.hppcpp/libcudf_streaming/include/cudf_streaming/channel_metadata.hppcpp/libcudf_streaming/include/cudf_streaming/detail/device_bloom_filter.hppcpp/libcudf_streaming/include/cudf_streaming/parquet.hppcpp/libcudf_streaming/include/cudf_streaming/partition.hppcpp/libcudf_streaming/include/cudf_streaming/partition_utils.hppcpp/libcudf_streaming/include/cudf_streaming/table_chunk.hppcpp/libcudf_streaming/include/cudf_streaming/utils.hppcpp/libcudf_streaming/src/bloom_filter.cppcpp/libcudf_streaming/src/channel_metadata.cppcpp/libcudf_streaming/src/detail/device_bloom_filter.cucpp/libcudf_streaming/src/parquet.cppcpp/libcudf_streaming/src/partition.cppcpp/libcudf_streaming/src/partition_utils.cppcpp/libcudf_streaming/src/table_chunk.cppcpp/libcudf_streaming/src/utils.cppcpp/libcudf_streaming/tests/streaming/test_allgather.cppcpp/libcudf_streaming/tests/streaming/test_channel_metadata.cppcpp/libcudf_streaming/tests/streaming/test_cudf_utils.cppcpp/libcudf_streaming/tests/streaming/test_leaf_actor.cppcpp/libcudf_streaming/tests/streaming/test_partition.cppcpp/libcudf_streaming/tests/streaming/test_read_parquet.cppcpp/libcudf_streaming/tests/streaming/test_shuffler.cppcpp/libcudf_streaming/tests/streaming/test_table_chunk.cppcpp/libcudf_streaming/tests/test_partition.cppcpp/libcudf_streaming/tests/test_shuffler.cppcpp/libcudf_streaming/tests/test_shuffler_many_streams.cpppython/cudf_polars/cudf_polars/engine/core.pypython/cudf_polars/cudf_polars/engine/dask.pypython/cudf_polars/cudf_polars/engine/ray.pypython/cudf_polars/cudf_polars/engine/spmd.pypython/cudf_polars/cudf_polars/experimental/benchmarks/pdsh_results.jsonlpython/cudf_polars/cudf_polars/streaming/actor_graph/collectives/allgather.pypython/cudf_polars/cudf_polars/streaming/actor_graph/collectives/shuffle.pypython/cudf_polars/cudf_polars/streaming/actor_graph/collectives/sort.pypython/cudf_polars/cudf_polars/streaming/actor_graph/core.pypython/cudf_polars/cudf_polars/streaming/actor_graph/groupby.pypython/cudf_polars/cudf_polars/streaming/actor_graph/io.pypython/cudf_polars/cudf_polars/streaming/actor_graph/join.pypython/cudf_polars/cudf_polars/streaming/actor_graph/nodes.pypython/cudf_polars/cudf_polars/streaming/actor_graph/over.pypython/cudf_polars/cudf_polars/streaming/actor_graph/repartition.pypython/cudf_polars/cudf_polars/streaming/actor_graph/tracing.pypython/cudf_polars/cudf_polars/streaming/actor_graph/union.pypython/cudf_polars/cudf_polars/streaming/actor_graph/utils.pypython/cudf_polars/tests/streaming/test_allgather.pypython/cudf_polars/tests/streaming/test_metadata.pypython/cudf_polars/tests/streaming/test_shuffler.pypython/cudf_polars/tests/streaming/test_spilling.pypython/cudf_polars/tests/streaming/test_tracing.pypython/cudf_streaming/CMakeLists.txtpython/cudf_streaming/cudf_streaming/__init__.pxdpython/cudf_streaming/cudf_streaming/__init__.pypython/cudf_streaming/cudf_streaming/bloom_filter.pxdpython/cudf_streaming/cudf_streaming/bloom_filter.pyipython/cudf_streaming/cudf_streaming/bloom_filter.pyxpython/cudf_streaming/cudf_streaming/channel_metadata.pxdpython/cudf_streaming/cudf_streaming/channel_metadata.pyipython/cudf_streaming/cudf_streaming/channel_metadata.pyxpython/cudf_streaming/cudf_streaming/examples/bulk_mpi_shuffle.pypython/cudf_streaming/cudf_streaming/examples/bulk_ray_shuffle.pypython/cudf_streaming/cudf_streaming/examples/ray_shuffle_example.pypython/cudf_streaming/cudf_streaming/examples/streaming_basic_example.pypython/cudf_streaming/cudf_streaming/integrations/CMakeLists.txtpython/cudf_streaming/cudf_streaming/integrations/__init__.pxdpython/cudf_streaming/cudf_streaming/integrations/__init__.pypython/cudf_streaming/cudf_streaming/parquet.pyipython/cudf_streaming/cudf_streaming/parquet.pyxpython/cudf_streaming/cudf_streaming/partition.pyipython/cudf_streaming/cudf_streaming/partition.pyxpython/cudf_streaming/cudf_streaming/partition_utils.pxdpython/cudf_streaming/cudf_streaming/partition_utils.pyipython/cudf_streaming/cudf_streaming/partition_utils.pyxpython/cudf_streaming/cudf_streaming/streaming/CMakeLists.txtpython/cudf_streaming/cudf_streaming/streaming/__init__.pypython/cudf_streaming/cudf_streaming/table_chunk.pxdpython/cudf_streaming/cudf_streaming/table_chunk.pyipython/cudf_streaming/cudf_streaming/table_chunk.pyxpython/cudf_streaming/cudf_streaming/tests/test_allgather.pypython/cudf_streaming/cudf_streaming/tests/test_bloom_filter.pypython/cudf_streaming/cudf_streaming/tests/test_channel_metadata.pypython/cudf_streaming/cudf_streaming/tests/test_integration_partition.pypython/cudf_streaming/cudf_streaming/tests/test_partition.pypython/cudf_streaming/cudf_streaming/tests/test_read_parquet.pypython/cudf_streaming/cudf_streaming/tests/test_shuffler.pypython/cudf_streaming/cudf_streaming/tests/test_sparse_alltoall.pypython/cudf_streaming/cudf_streaming/tests/test_streaming_allgather.pypython/cudf_streaming/cudf_streaming/tests/test_streaming_define_actor.pypython/cudf_streaming/cudf_streaming/tests/test_streaming_fanout.pypython/cudf_streaming/cudf_streaming/tests/test_streaming_leaf_actor.pypython/cudf_streaming/cudf_streaming/tests/test_streaming_shuffler.pypython/cudf_streaming/cudf_streaming/tests/test_streaming_sparse_alltoall.pypython/cudf_streaming/cudf_streaming/tests/test_table_chunk.py
💤 Files with no reviewable changes (5)
- python/cudf_streaming/cudf_streaming/integrations/init.pxd
- python/cudf_streaming/cudf_streaming/integrations/CMakeLists.txt
- python/cudf_streaming/cudf_streaming/streaming/CMakeLists.txt
- python/cudf_streaming/cudf_streaming/integrations/init.py
- python/cudf_streaming/cudf_streaming/streaming/init.py
- Flatten cudf_streaming::streaming:: and cudf_streaming::integrations:: into cudf_streaming:: (preserving cudf_streaming::actor::) - Rename PascalCase types to snake_case: BloomFilter -> bloom_filter, TableChunk -> table_chunk, ChannelMetadata -> channel_metadata, etc. - Privatize integrations::BloomFilter as cudf_streaming::detail::device_bloom_filter - Flatten Python/Cython sub-packages: cudf_streaming.streaming.* and cudf_streaming.integrations.* -> cudf_streaming.* - Update all downstream imports in cudf_polars - Python class names remain PascalCase (only C++ types renamed) - No deprecation aliases (hard break)
832712c to
cfab80c
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cpp/libcudf_streaming/include/cudf_streaming/channel_metadata.hpp (1)
23-250:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftCRITICAL: Public API hard break without a deprecation bridge
This header replaces public symbols (
cudf_streaming::streaming::*and PascalCase types) with new top-level snake_case names but provides no deprecated aliases/wrappers. That creates immediate source-breaking changes for existing consumers ofcpp/includeAPIs.Suggested compatibility bridge
namespace cudf_streaming { // new API... } +namespace cudf_streaming::streaming { +using HashScheme [[deprecated("Use cudf_streaming::hash_scheme")]] = ::cudf_streaming::hash_scheme; +using OrderKey [[deprecated("Use cudf_streaming::order_key")]] = ::cudf_streaming::order_key; +using OrderScheme [[deprecated("Use cudf_streaming::order_scheme")]] = ::cudf_streaming::order_scheme; +using PartitioningSpec [[deprecated("Use cudf_streaming::partitioning_spec")]] = ::cudf_streaming::partitioning_spec; +using Partitioning [[deprecated("Use cudf_streaming::partitioning")]] = ::cudf_streaming::partitioning; +using ChannelMetadata [[deprecated("Use cudf_streaming::channel_metadata")]] = ::cudf_streaming::channel_metadata; +}As per coding guidelines, “C++ API changes without proper deprecation warnings” in public headers must include a deprecation cycle before removal.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cpp/libcudf_streaming/include/cudf_streaming/channel_metadata.hpp` around lines 23 - 250, The header is introducing breaking changes to public API symbols by moving/renaming them (e.g., from PascalCase to snake_case, from nested `cudf_streaming::streaming::*` to top-level) without providing backward compatibility aliases. Add deprecated alias declarations for all old symbol names that point to their new equivalents. For each struct or type that has been renamed or moved (such as `hash_scheme`, `order_key`, `order_scheme`, `partitioning_spec`, `partitioning`, `channel_metadata`, and the `to_message` function), create a deprecated using-alias or typedef with an appropriate deprecation attribute (e.g., `[[deprecated("Use new_name instead")]]`) to ensure existing code continues to compile while warning users to migrate to the new names. This deprecation bridge allows a transition period before the old symbols are eventually removed.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@cpp/libcudf_streaming/include/cudf_streaming/channel_metadata.hpp`:
- Around line 23-250: The header is introducing breaking changes to public API
symbols by moving/renaming them (e.g., from PascalCase to snake_case, from
nested `cudf_streaming::streaming::*` to top-level) without providing backward
compatibility aliases. Add deprecated alias declarations for all old symbol
names that point to their new equivalents. For each struct or type that has been
renamed or moved (such as `hash_scheme`, `order_key`, `order_scheme`,
`partitioning_spec`, `partitioning`, `channel_metadata`, and the `to_message`
function), create a deprecated using-alias or typedef with an appropriate
deprecation attribute (e.g., `[[deprecated("Use new_name instead")]]`) to ensure
existing code continues to compile while warning users to migrate to the new
names. This deprecation bridge allows a transition period before the old symbols
are eventually removed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 8c97f536-1aff-4340-9e1b-cd6b4c957167
📒 Files selected for processing (116)
cpp/libcudf_streaming/CMakeLists.txtcpp/libcudf_streaming/benchmarks/bench_partition.cppcpp/libcudf_streaming/benchmarks/bench_shuffle.cppcpp/libcudf_streaming/benchmarks/streaming/bench_streaming_shuffle.cppcpp/libcudf_streaming/benchmarks/streaming/data_generator.hppcpp/libcudf_streaming/benchmarks/streaming/ndsh/bench_read.cppcpp/libcudf_streaming/benchmarks/streaming/ndsh/concatenate.cppcpp/libcudf_streaming/benchmarks/streaming/ndsh/concatenate.hppcpp/libcudf_streaming/benchmarks/streaming/ndsh/groupby.cppcpp/libcudf_streaming/benchmarks/streaming/ndsh/groupby.hppcpp/libcudf_streaming/benchmarks/streaming/ndsh/join.cppcpp/libcudf_streaming/benchmarks/streaming/ndsh/join.hppcpp/libcudf_streaming/benchmarks/streaming/ndsh/parquet_writer.cppcpp/libcudf_streaming/benchmarks/streaming/ndsh/parquet_writer.hppcpp/libcudf_streaming/benchmarks/streaming/ndsh/q01.cppcpp/libcudf_streaming/benchmarks/streaming/ndsh/q03.cppcpp/libcudf_streaming/benchmarks/streaming/ndsh/q04.cppcpp/libcudf_streaming/benchmarks/streaming/ndsh/q09.cppcpp/libcudf_streaming/benchmarks/streaming/ndsh/q21.cppcpp/libcudf_streaming/benchmarks/streaming/ndsh/sort.cppcpp/libcudf_streaming/benchmarks/streaming/ndsh/sort.hppcpp/libcudf_streaming/benchmarks/streaming/ndsh/utils.cppcpp/libcudf_streaming/benchmarks/streaming/ndsh/utils.hppcpp/libcudf_streaming/examples/example_shuffle.cppcpp/libcudf_streaming/include/cudf_streaming/bloom_filter.hppcpp/libcudf_streaming/include/cudf_streaming/channel_metadata.hppcpp/libcudf_streaming/include/cudf_streaming/detail/device_bloom_filter.hppcpp/libcudf_streaming/include/cudf_streaming/parquet.hppcpp/libcudf_streaming/include/cudf_streaming/partition.hppcpp/libcudf_streaming/include/cudf_streaming/partition_utils.hppcpp/libcudf_streaming/include/cudf_streaming/table_chunk.hppcpp/libcudf_streaming/include/cudf_streaming/utils.hppcpp/libcudf_streaming/src/bloom_filter.cppcpp/libcudf_streaming/src/channel_metadata.cppcpp/libcudf_streaming/src/detail/device_bloom_filter.cucpp/libcudf_streaming/src/parquet.cppcpp/libcudf_streaming/src/partition.cppcpp/libcudf_streaming/src/partition_utils.cppcpp/libcudf_streaming/src/table_chunk.cppcpp/libcudf_streaming/src/utils.cppcpp/libcudf_streaming/tests/streaming/test_allgather.cppcpp/libcudf_streaming/tests/streaming/test_channel_metadata.cppcpp/libcudf_streaming/tests/streaming/test_cudf_utils.cppcpp/libcudf_streaming/tests/streaming/test_leaf_actor.cppcpp/libcudf_streaming/tests/streaming/test_partition.cppcpp/libcudf_streaming/tests/streaming/test_read_parquet.cppcpp/libcudf_streaming/tests/streaming/test_shuffler.cppcpp/libcudf_streaming/tests/streaming/test_table_chunk.cppcpp/libcudf_streaming/tests/test_partition.cppcpp/libcudf_streaming/tests/test_shuffler.cppcpp/libcudf_streaming/tests/test_shuffler_many_streams.cpppython/cudf_polars/cudf_polars/engine/core.pypython/cudf_polars/cudf_polars/engine/dask.pypython/cudf_polars/cudf_polars/engine/ray.pypython/cudf_polars/cudf_polars/engine/spmd.pypython/cudf_polars/cudf_polars/streaming/actor_graph/collectives/allgather.pypython/cudf_polars/cudf_polars/streaming/actor_graph/collectives/shuffle.pypython/cudf_polars/cudf_polars/streaming/actor_graph/collectives/sort.pypython/cudf_polars/cudf_polars/streaming/actor_graph/core.pypython/cudf_polars/cudf_polars/streaming/actor_graph/groupby.pypython/cudf_polars/cudf_polars/streaming/actor_graph/io.pypython/cudf_polars/cudf_polars/streaming/actor_graph/join.pypython/cudf_polars/cudf_polars/streaming/actor_graph/nodes.pypython/cudf_polars/cudf_polars/streaming/actor_graph/over.pypython/cudf_polars/cudf_polars/streaming/actor_graph/repartition.pypython/cudf_polars/cudf_polars/streaming/actor_graph/tracing.pypython/cudf_polars/cudf_polars/streaming/actor_graph/union.pypython/cudf_polars/cudf_polars/streaming/actor_graph/utils.pypython/cudf_polars/tests/streaming/test_allgather.pypython/cudf_polars/tests/streaming/test_metadata.pypython/cudf_polars/tests/streaming/test_shuffler.pypython/cudf_polars/tests/streaming/test_spilling.pypython/cudf_polars/tests/streaming/test_tracing.pypython/cudf_streaming/CMakeLists.txtpython/cudf_streaming/cudf_streaming/__init__.pxdpython/cudf_streaming/cudf_streaming/__init__.pypython/cudf_streaming/cudf_streaming/bloom_filter.pxdpython/cudf_streaming/cudf_streaming/bloom_filter.pyipython/cudf_streaming/cudf_streaming/bloom_filter.pyxpython/cudf_streaming/cudf_streaming/channel_metadata.pxdpython/cudf_streaming/cudf_streaming/channel_metadata.pyipython/cudf_streaming/cudf_streaming/channel_metadata.pyxpython/cudf_streaming/cudf_streaming/examples/bulk_mpi_shuffle.pypython/cudf_streaming/cudf_streaming/examples/bulk_ray_shuffle.pypython/cudf_streaming/cudf_streaming/examples/ray_shuffle_example.pypython/cudf_streaming/cudf_streaming/examples/streaming_basic_example.pypython/cudf_streaming/cudf_streaming/integrations/CMakeLists.txtpython/cudf_streaming/cudf_streaming/integrations/__init__.pxdpython/cudf_streaming/cudf_streaming/integrations/__init__.pypython/cudf_streaming/cudf_streaming/parquet.pyipython/cudf_streaming/cudf_streaming/parquet.pyxpython/cudf_streaming/cudf_streaming/partition.pyipython/cudf_streaming/cudf_streaming/partition.pyxpython/cudf_streaming/cudf_streaming/partition_utils.pxdpython/cudf_streaming/cudf_streaming/partition_utils.pyipython/cudf_streaming/cudf_streaming/partition_utils.pyxpython/cudf_streaming/cudf_streaming/streaming/CMakeLists.txtpython/cudf_streaming/cudf_streaming/streaming/__init__.pypython/cudf_streaming/cudf_streaming/table_chunk.pxdpython/cudf_streaming/cudf_streaming/table_chunk.pyipython/cudf_streaming/cudf_streaming/table_chunk.pyxpython/cudf_streaming/cudf_streaming/tests/test_allgather.pypython/cudf_streaming/cudf_streaming/tests/test_bloom_filter.pypython/cudf_streaming/cudf_streaming/tests/test_channel_metadata.pypython/cudf_streaming/cudf_streaming/tests/test_integration_partition.pypython/cudf_streaming/cudf_streaming/tests/test_partition.pypython/cudf_streaming/cudf_streaming/tests/test_read_parquet.pypython/cudf_streaming/cudf_streaming/tests/test_shuffler.pypython/cudf_streaming/cudf_streaming/tests/test_sparse_alltoall.pypython/cudf_streaming/cudf_streaming/tests/test_streaming_allgather.pypython/cudf_streaming/cudf_streaming/tests/test_streaming_define_actor.pypython/cudf_streaming/cudf_streaming/tests/test_streaming_fanout.pypython/cudf_streaming/cudf_streaming/tests/test_streaming_leaf_actor.pypython/cudf_streaming/cudf_streaming/tests/test_streaming_shuffler.pypython/cudf_streaming/cudf_streaming/tests/test_streaming_sparse_alltoall.pypython/cudf_streaming/cudf_streaming/tests/test_table_chunk.py
💤 Files with no reviewable changes (5)
- python/cudf_streaming/cudf_streaming/integrations/init.pxd
- python/cudf_streaming/cudf_streaming/streaming/CMakeLists.txt
- python/cudf_streaming/cudf_streaming/streaming/init.py
- python/cudf_streaming/cudf_streaming/integrations/CMakeLists.txt
- python/cudf_streaming/cudf_streaming/integrations/init.py
✅ Files skipped from review due to trivial changes (31)
- python/cudf_streaming/cudf_streaming/parquet.pyi
- cpp/libcudf_streaming/benchmarks/streaming/ndsh/parquet_writer.hpp
- python/cudf_streaming/cudf_streaming/tests/test_streaming_define_actor.py
- cpp/libcudf_streaming/benchmarks/streaming/ndsh/sort.hpp
- python/cudf_streaming/cudf_streaming/bloom_filter.pyi
- python/cudf_streaming/cudf_streaming/channel_metadata.pyi
- python/cudf_streaming/cudf_streaming/tests/test_streaming_sparse_alltoall.py
- python/cudf_polars/cudf_polars/streaming/actor_graph/repartition.py
- python/cudf_polars/cudf_polars/streaming/actor_graph/join.py
- cpp/libcudf_streaming/tests/streaming/test_allgather.cpp
- cpp/libcudf_streaming/benchmarks/streaming/ndsh/groupby.hpp
- python/cudf_polars/cudf_polars/streaming/actor_graph/utils.py
- python/cudf_streaming/cudf_streaming/tests/test_sparse_alltoall.py
- python/cudf_streaming/cudf_streaming/tests/test_partition.py
- python/cudf_polars/cudf_polars/streaming/actor_graph/collectives/shuffle.py
- python/cudf_polars/cudf_polars/streaming/actor_graph/core.py
- python/cudf_polars/cudf_polars/engine/spmd.py
- python/cudf_polars/cudf_polars/streaming/actor_graph/collectives/allgather.py
- python/cudf_streaming/cudf_streaming/tests/test_read_parquet.py
- python/cudf_polars/tests/streaming/test_tracing.py
- python/cudf_streaming/cudf_streaming/tests/test_streaming_allgather.py
- python/cudf_streaming/cudf_streaming/partition.pyi
- python/cudf_polars/tests/streaming/test_shuffler.py
- python/cudf_streaming/cudf_streaming/examples/ray_shuffle_example.py
- python/cudf_polars/cudf_polars/engine/core.py
- cpp/libcudf_streaming/tests/test_partition.cpp
- python/cudf_polars/tests/streaming/test_metadata.py
- python/cudf_polars/cudf_polars/streaming/actor_graph/union.py
- python/cudf_streaming/cudf_streaming/tests/test_shuffler.py
- python/cudf_streaming/cudf_streaming/tests/test_streaming_fanout.py
- python/cudf_polars/cudf_polars/streaming/actor_graph/collectives/sort.py
🚧 Files skipped from review as they are similar to previous changes (65)
- cpp/libcudf_streaming/benchmarks/streaming/ndsh/concatenate.hpp
- python/cudf_polars/tests/streaming/test_allgather.py
- cpp/libcudf_streaming/include/cudf_streaming/utils.hpp
- python/cudf_streaming/cudf_streaming/tests/test_streaming_leaf_actor.py
- python/cudf_polars/cudf_polars/engine/ray.py
- python/cudf_streaming/cudf_streaming/examples/bulk_ray_shuffle.py
- python/cudf_polars/cudf_polars/streaming/actor_graph/tracing.py
- python/cudf_streaming/cudf_streaming/examples/bulk_mpi_shuffle.py
- python/cudf_streaming/cudf_streaming/table_chunk.pxd
- python/cudf_streaming/cudf_streaming/tests/test_bloom_filter.py
- python/cudf_polars/cudf_polars/engine/dask.py
- cpp/libcudf_streaming/benchmarks/streaming/ndsh/groupby.cpp
- python/cudf_streaming/cudf_streaming/tests/test_channel_metadata.py
- cpp/libcudf_streaming/benchmarks/streaming/data_generator.hpp
- cpp/libcudf_streaming/CMakeLists.txt
- python/cudf_polars/cudf_polars/streaming/actor_graph/nodes.py
- cpp/libcudf_streaming/src/utils.cpp
- cpp/libcudf_streaming/benchmarks/streaming/ndsh/parquet_writer.cpp
- python/cudf_streaming/cudf_streaming/tests/test_integration_partition.py
- python/cudf_streaming/cudf_streaming/init.pxd
- python/cudf_streaming/cudf_streaming/init.py
- python/cudf_streaming/cudf_streaming/bloom_filter.pyx
- python/cudf_streaming/cudf_streaming/channel_metadata.pyx
- cpp/libcudf_streaming/benchmarks/bench_partition.cpp
- cpp/libcudf_streaming/src/partition_utils.cpp
- cpp/libcudf_streaming/tests/streaming/test_shuffler.cpp
- cpp/libcudf_streaming/benchmarks/bench_shuffle.cpp
- python/cudf_streaming/cudf_streaming/tests/test_streaming_shuffler.py
- python/cudf_streaming/cudf_streaming/bloom_filter.pxd
- cpp/libcudf_streaming/examples/example_shuffle.cpp
- cpp/libcudf_streaming/tests/streaming/test_leaf_actor.cpp
- cpp/libcudf_streaming/benchmarks/streaming/ndsh/sort.cpp
- cpp/libcudf_streaming/benchmarks/streaming/bench_streaming_shuffle.cpp
- python/cudf_streaming/cudf_streaming/tests/test_allgather.py
- cpp/libcudf_streaming/src/partition.cpp
- cpp/libcudf_streaming/benchmarks/streaming/ndsh/join.hpp
- cpp/libcudf_streaming/benchmarks/streaming/ndsh/utils.hpp
- python/cudf_streaming/CMakeLists.txt
- python/cudf_polars/tests/streaming/test_spilling.py
- cpp/libcudf_streaming/include/cudf_streaming/partition_utils.hpp
- cpp/libcudf_streaming/benchmarks/streaming/ndsh/bench_read.cpp
- python/cudf_streaming/cudf_streaming/partition.pyx
- cpp/libcudf_streaming/include/cudf_streaming/parquet.hpp
- cpp/libcudf_streaming/include/cudf_streaming/detail/device_bloom_filter.hpp
- python/cudf_streaming/cudf_streaming/tests/test_table_chunk.py
- python/cudf_streaming/cudf_streaming/partition_utils.pyx
- python/cudf_streaming/cudf_streaming/parquet.pyx
- cpp/libcudf_streaming/tests/streaming/test_partition.cpp
- cpp/libcudf_streaming/tests/streaming/test_channel_metadata.cpp
- cpp/libcudf_streaming/include/cudf_streaming/partition.hpp
- cpp/libcudf_streaming/benchmarks/streaming/ndsh/q09.cpp
- cpp/libcudf_streaming/src/detail/device_bloom_filter.cu
- python/cudf_streaming/cudf_streaming/table_chunk.pyx
- cpp/libcudf_streaming/include/cudf_streaming/bloom_filter.hpp
- cpp/libcudf_streaming/tests/test_shuffler.cpp
- cpp/libcudf_streaming/src/parquet.cpp
- cpp/libcudf_streaming/benchmarks/streaming/ndsh/q03.cpp
- cpp/libcudf_streaming/src/bloom_filter.cpp
- python/cudf_streaming/cudf_streaming/channel_metadata.pxd
- cpp/libcudf_streaming/tests/streaming/test_read_parquet.cpp
- cpp/libcudf_streaming/benchmarks/streaming/ndsh/q21.cpp
- cpp/libcudf_streaming/benchmarks/streaming/ndsh/join.cpp
- cpp/libcudf_streaming/src/channel_metadata.cpp
- cpp/libcudf_streaming/include/cudf_streaming/table_chunk.hpp
- cpp/libcudf_streaming/src/table_chunk.cpp
|
@coderabbitai Thanks for the review. The deprecation bridge suggestions are not applicable here — |
|
Tip For best results, initiate chat on the files or code changes.
✏️ Learnings added
|
…pace_casing # Conflicts: # cpp/libcudf_streaming/benchmarks/streaming/ndsh/q03.cpp # cpp/libcudf_streaming/benchmarks/streaming/ndsh/q21.cpp # cpp/libcudf_streaming/tests/streaming/test_allgather.cpp # cpp/libcudf_streaming/tests/streaming/test_shuffler.cpp # cpp/libcudf_streaming/tests/test_shuffler.cpp # cpp/libcudf_streaming/tests/test_shuffler_many_streams.cpp
|
/merge |
…DIA#22909) Flatten the `libcudf_streaming` namespace hierarchy and rename PascalCase C++ types to snake_case. ### Changes **C++ Namespace Flattening:** - `cudf_streaming::streaming::` → `cudf_streaming::` - `cudf_streaming::integrations::` → `cudf_streaming::` - `cudf_streaming::actor::` preserved (intentional grouping) **C++ Type Renames (snake_case):** - `BloomFilter` → `bloom_filter` - `TableChunk` → `table_chunk` - `ChannelMetadata` → `channel_metadata` - `PartitioningSpec` → `partitioning_spec` - `BloomFilterChunk` → `bloom_filter_chunk` **Detail Privatization:** - `integrations::BloomFilter` (device-level) → `cudf_streaming::detail::device_bloom_filter` - Header moved to `include/cudf_streaming/detail/device_bloom_filter.hpp` **Directory Flattening:** - C++ headers: `include/cudf_streaming/streaming/` and `include/cudf_streaming/integrations/` → `include/cudf_streaming/` - C++ sources: `src/streaming/` and `src/integrations/` → `src/` (and `src/detail/`) - Python/Cython: `cudf_streaming.streaming.*` and `cudf_streaming.integrations.*` → `cudf_streaming.*` **Downstream Updates:** - All `cudf_polars` imports updated to new paths ### Notes - Python class names remain PascalCase (only C++ types renamed) - No deprecation aliases — hard break - All C++ tests pass (420 pass, 2 pre-existing OOM failures) - All Python tests pass (cudf_streaming: 386 pass, cudf_polars streaming: 1133 pass) Authors: - Vyas Ramasubramani (https://github.com/vyasr) Approvers: - Mads R. B. Kristensen (https://github.com/madsbk) - Lawrence Mitchell (https://github.com/wence-) - Peter Andreas Entschev (https://github.com/pentschev) - Muhammad Haseeb (https://github.com/mhaseeb123) - Matthew Roeschke (https://github.com/mroeschke) URL: NVIDIA#22909
Description
Flatten the
libcudf_streamingnamespace hierarchy and rename PascalCase C++ types to snake_case.Changes
C++ Namespace Flattening:
cudf_streaming::streaming::→cudf_streaming::cudf_streaming::integrations::→cudf_streaming::cudf_streaming::actor::preserved (intentional grouping)C++ Type Renames (snake_case):
BloomFilter→bloom_filterTableChunk→table_chunkChannelMetadata→channel_metadataPartitioningSpec→partitioning_specBloomFilterChunk→bloom_filter_chunkDetail Privatization:
integrations::BloomFilter(device-level) →cudf_streaming::detail::device_bloom_filterinclude/cudf_streaming/detail/device_bloom_filter.hppDirectory Flattening:
include/cudf_streaming/streaming/andinclude/cudf_streaming/integrations/→include/cudf_streaming/src/streaming/andsrc/integrations/→src/(andsrc/detail/)cudf_streaming.streaming.*andcudf_streaming.integrations.*→cudf_streaming.*Downstream Updates:
cudf_polarsimports updated to new pathsNotes