Warn and resolve conflict when both blocksize and files_per_partition are specified

[ArivunidhiA]

Description

Closes #1401

When both blocksize and files_per_partition are specified in FilePartitioningStage, the stage calls _get_file_list_with_sizes() (returning list[tuple[str, int]]) but then passes the result to _partition_by_count() which expects list[str], causing a confusing ValueError: Invalid file path or buffer object type: <class 'tuple'>.

This adds an early check in __post_init__ that logs a warning and sets blocksize = None when both are provided, so files_per_partition takes precedence cleanly.

Usage

# Previously crashed with a cryptic tuple error:
FilePartitioningStage(
    file_paths="/data",
    files_per_partition=1,
    blocksize="128MB",
)
# Now logs: "Both 'files_per_partition' and 'blocksize' were specified.
#            'files_per_partition' will take precedence and 'blocksize' will be ignored."

Checklist

• I am familiar with the Contributing Guide.
• New or Existing tests cover these changes.
• The documentation is up to date with these changes.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[greptile-apps]

Greptile Summary

This PR fixes a bug (#1401) where specifying both blocksize and files_per_partition in FilePartitioningStage caused a confusing ValueError due to type mismatch—_get_file_list_with_sizes() returned list[tuple[str, int]] but _partition_by_count() expected list[str].

The fix adds an early conflict resolution in __post_init__ that logs a warning and sets blocksize = None when both are provided, giving files_per_partition precedence.

• Added conflict detection in __post_init__ with a loguru warning and blocksize = None fallback
• Split the existing test_initialization_custom_values test into two separate tests (one per partitioning strategy) to avoid triggering the new warning
• Added test_both_blocksize_and_files_per_partition_warns to verify the warning behavior and blocksize nullification

Confidence Score: 5/5

• This PR is safe to merge — it's a small, well-scoped defensive fix with proper test coverage.
• The change is minimal (6 lines of production code), correctly addresses the root cause of the reported bug, and includes comprehensive test coverage. The logic is straightforward — an early guard clause in __post_init__ that resolves the conflict before any downstream code runs. No existing behavior is changed for users who only specify one of the two parameters.
• No files require special attention.

Important Files Changed

Filename	Overview
nemo_curator/stages/file_partitioning.py	Added early conflict detection in __post_init__ when both blocksize and files_per_partition are set, logging a warning and nullifying blocksize. Clean, minimal fix that correctly prevents the downstream type mismatch bug.
tests/stages/common/test_file_partitioning.py	Split the original combined initialization test into two separate tests (one per partitioning mode) and added a dedicated test for the new warning behavior using caplog. Tests are well-structured and consistent with the project's existing test patterns.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["FilePartitioningStage.__post_init__()"] --> B{"Both files_per_partition\nand blocksize set?"}
    B -->|Yes| C["Log warning\nSet blocksize = None"]
    B -->|No| D["Continue initialization"]
    C --> D
    D --> E["process() called"]
    E --> F{"blocksize is set?"}
    F -->|Yes| G["_get_file_list_with_sizes()\nReturns list[tuple[str, int]]"]
    F -->|No| H["_get_file_list()\nReturns list[str]"]
    G --> I{"files_per_partition set?"}
    H --> I
    I -->|Yes| J["_partition_by_count(files)\nExpects list[str]"]
    I -->|No| K{"blocksize set?"}
    K -->|Yes| L["_partition_by_size(files)\nExpects list[tuple[str, int]]"]
    K -->|No| M["Default: 1 file per partition"]
    J --> N["Create FileGroupTasks"]
    L --> N
    M --> N

Loading

_{Last reviewed commit: 93a0d4b}

[greptile-apps]

_{2 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

[sarahyurick]

Thanks @ArivunidhiA ! Added a request.

[sarahyurick]

Can you add a check to ensure that the warning was raised too?

[ArivunidhiA]

Added a caplog assertion to verify the warning is raised. Thanks for the review!

[greptile-apps]

_{2 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

_{2 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

[sarahyurick]

/ok to test 93a0d4b

[sarahyurick]

LGTM, thank you @ArivunidhiA !

[ArivunidhiA]

Thanks for the review, I appreciate it!