Skip to content

Conversation

@JanuszL
Copy link
Contributor

@JanuszL JanuszL commented Dec 4, 2025

  • Previously, weekly/nightly builds were differentiated by flavor suffix in the
    package name (e.g., nvidia-dali-weekly-cuda*, nvidia-dali-nightly-cuda*).
    This change moves to a unified naming schema where:
    • Package names are consistent regardless of build type (nvidia-dali-cuda*)
    • Dev builds are differentiated by .dev{TIMESTAMP} suffix in version number
    • Stable and dev packages share the same package name, distinguished only by version
  • Removes DALI_FLAVOR_MINUS variable from CMakeLists.txt files
  • Updates package names to remove flavor suffix:
    • nvidia-dali-cuda* (was nvidia-dali-{flavor}-cuda*)
    • nvidia-dali-tf-plugin-cuda* (was nvidia-dali-tf-plugin-{flavor}-cuda*)
    • nvidia-dali-{plugin} (was nvidia-dali-{plugin}-{flavor})
  • Standardizes dev version format: use .dev{TIMESTAMP} instead of .{TIMESTAMP}
  • Updates install_requires in TF plugin to match new naming schema
  • Updates installation documentation to explain new distribution method:
    • Documents new unified package naming with .dev{TIMESTAMP} version suffix
    • Explains use of --pre flag for installing dev builds
    • Adds legacy distribution section for pre-1.53 builds
    • Clarifies nightly vs weekly build schedule and availability

Category:

Other (e.g. Documentation, Tests, Configuration)

Description:

  • Previously, weekly/nightly builds were differentiated by flavor suffix in the
    package name (e.g., nvidia-dali-weekly-cuda*, nvidia-dali-nightly-cuda*).
    This change moves to a unified naming schema where:
    • Package names are consistent regardless of build type (nvidia-dali-cuda*)
    • Dev builds are differentiated by .dev{TIMESTAMP} suffix in version number
    • Stable and dev packages share the same package name, distinguished only by version
  • Removes DALI_FLAVOR_MINUS variable from CMakeLists.txt files
  • Updates package names to remove flavor suffix:
    • nvidia-dali-cuda* (was nvidia-dali-{flavor}-cuda*)
    • nvidia-dali-tf-plugin-cuda* (was nvidia-dali-tf-plugin-{flavor}-cuda*)
    • nvidia-dali-{plugin} (was nvidia-dali-{plugin}-{flavor})
  • Standardizes dev version format: use .dev{TIMESTAMP} instead of .{TIMESTAMP}
  • Updates install_requires in TF plugin to match new naming schema
  • Updates installation documentation to explain new distribution method:
    • Documents new unified package naming with .dev{TIMESTAMP} version suffix
    • Explains use of --pre flag for installing dev builds
    • Adds legacy distribution section for pre-1.53 builds
    • Clarifies nightly vs weekly build schedule and availability

Additional information:

Affected modules and functionalities:

  • cmake files
  • setup.py.in files

Key points relevant for the review:

  • NA

Tests:

  • Existing tests apply
    • nightly build runs
  • New tests added
    • Python tests
    • GTests
    • Benchmark
    • Other
  • N/A

Checklist

Documentation

  • Existing documentation applies
  • Documentation updated
    • Docstring
    • Doxygen
    • RST
    • Jupyter
    • Other
  • N/A

DALI team only

Requirements

  • Implements new requirements
  • Affects existing requirements
  • N/A

REQ IDs: N/A

JIRA TASK: N/A

@greptile-apps
Copy link

greptile-apps bot commented Dec 4, 2025

Greptile Overview

Greptile Summary

Refactored DALI package naming from flavor-based suffixes to unified naming with version-based differentiation using PEP 440 dev releases.

  • Removed DALI_FLAVOR_MINUS variable from all CMakeLists.txt files
  • Unified package names: nvidia-dali-cuda* and nvidia-dali-tf-plugin-cuda* (removed -nightly/-weekly suffixes)
  • Changed version format from .{TIMESTAMP} to direct {TIMESTAMP} append (e.g., 1.53.0dev20250104)
  • Added CMake validation to ensure dev builds have 'dev' in VERSION file
  • Updated installation docs with new --pre flag instructions and legacy section for pre-1.53 builds
  • Fixed plugins/CMakeLists.txt to actually apply TIMESTAMP (was previously a no-op)

Confidence Score: 4/5

  • Safe to merge with minor documentation inconsistency in compilation.rst
  • Implementation is sound with proper validation and PEP 440-compliant versioning. The CMake changes correctly remove DALI_FLAVOR_MINUS and implement dev version validation. However, docs/compilation.rst was not updated to reflect the new naming convention, creating a minor inconsistency in documentation.
  • docs/compilation.rst should be updated to reflect new unified package naming (currently references outdated flavor-based names)

Important Files Changed

File Analysis

Filename Score Overview
dali/python/CMakeLists.txt 5/5 Removed DALI_FLAVOR_MINUS, changed version format from .{TIMESTAMP} to {TIMESTAMP}, added validation to ensure dev builds have 'dev' in VERSION
dali/python/setup.py.in 5/5 Updated package name from nvidia-dali@DALI_FLAVOR_MINUS@-cuda* to nvidia-dali-cuda*, removing flavor suffix from package name
dali_tf_plugin/setup.py.in 5/5 Updated TF plugin package name and dependency from flavor-suffixed to unified naming (nvidia-dali-tf-plugin-cuda* and nvidia-dali-cuda*)
docs/installation.rst 4/5 Updated installation docs with new unified package naming, added --pre flag instructions, reorganized nightly/weekly sections, added legacy section for pre-1.53 builds

Sequence Diagram

sequenceDiagram
    participant Build as Build System
    participant CMake as CMake
    participant Setup as setup.py
    participant PyPI as PyPI/Index
    participant User as End User
    
    Note over Build,CMake: Build Configuration Phase
    Build->>CMake: Set DALI_BUILD_FLAVOR (e.g., "nightly")
    Build->>CMake: Set TIMESTAMP (e.g., "20250104")
    CMake->>CMake: Read VERSION file (e.g., "1.53.0dev")
    alt Development Build (DALI_BUILD_FLAVOR set)
        CMake->>CMake: Validate VERSION contains "dev"
        CMake->>CMake: Append TIMESTAMP: "1.53.0dev20250104"
    else Stable Build
        CMake->>CMake: Keep VERSION as-is: "1.53.0"
    end
    
    Note over CMake,Setup: Package Configuration Phase
    CMake->>Setup: Configure setup.py with unified name
    Note right of Setup: OLD: nvidia-dali-nightly-cuda120<br/>NEW: nvidia-dali-cuda120
    Setup->>Setup: Set version to "1.53.0dev20250104"
    
    Note over Setup,PyPI: Publishing Phase
    Setup->>PyPI: Publish package
    Note right of PyPI: Package: nvidia-dali-cuda120<br/>Version: 1.53.0dev20250104<br/>PEP 440 dev release
    
    Note over User,PyPI: Installation Phase
    User->>PyPI: pip install --pre nvidia-dali-cuda120
    PyPI->>User: Return dev version (1.53.0dev20250104)
    User->>PyPI: pip install nvidia-dali-cuda120
    PyPI->>User: Return stable version only (1.53.0)
Loading

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Additional Comments (4)

  1. docs/installation.rst, line 145-146 (link)

    logic: package names in documentation still reference old flavor-based naming (nvidia-dali-nightly-cuda120, nvidia-dali-tf-plugin-nightly-cuda120), should be updated to new unified naming (nvidia-dali-cuda120, nvidia-dali-tf-plugin-cuda120)

  2. docs/installation.rst, line 152-153 (link)

    logic: package names in documentation still reference old flavor-based naming (nvidia-dali-nightly-cuda130, nvidia-dali-tf-plugin-nightly-cuda130), should be updated to new unified naming (nvidia-dali-cuda130, nvidia-dali-tf-plugin-cuda130)

  3. docs/installation.rst, line 164-165 (link)

    logic: package names in documentation still reference old flavor-based naming (nvidia-dali-weekly-cuda130, nvidia-dali-tf-plugin-weekly-cuda130), should be updated to new unified naming (nvidia-dali-cuda130, nvidia-dali-tf-plugin-cuda130)

  4. docs/compilation.rst, line 72-73 (link)

    logic: documentation mentions old naming convention where DALI_BUILD_FLAVOR adds suffix to package name (e.g., nvidia-dali-nightly), but this has changed to version-based differentiation

6 files reviewed, 4 comments

Edit Code Review Agent Settings | Greptile

@JanuszL JanuszL force-pushed the new_dev_naming_schema branch 2 times, most recently from 3e1fa28 to 2a28fbe Compare December 4, 2025 08:03
@JanuszL
Copy link
Contributor Author

JanuszL commented Dec 4, 2025

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [39593786]: BUILD STARTED

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

7 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile


It is recommended to uninstall regular DALI and TensorFlow plugin before installing nightly or weekly
builds as they are installed in the same path
**New distribution method (starting from version 2.0):** Nightly and weekly builds now use the same
Copy link

Choose a reason for hiding this comment

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

logic: Version mismatch: documentation states "starting from version 2.0" but the current VERSION file shows 1.53.0dev. Should this say "starting from version 1.53" instead?

Suggested change
**New distribution method (starting from version 2.0):** Nightly and weekly builds now use the same
**New distribution method (starting from version 1.53):** Nightly and weekly builds now use the same

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I should be fixed now.

@JanuszL JanuszL force-pushed the new_dev_naming_schema branch from 2a28fbe to 2daff9a Compare December 4, 2025 08:17
@JanuszL JanuszL marked this pull request as draft December 4, 2025 08:19
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Additional Comments (1)

  1. docs/compilation.rst, line 72-73 (link)

    style: Outdated documentation - still references old package naming (e.g., nvidia-dali-nightly). Should be updated to reflect new unified naming where flavor no longer affects package name, only version suffix.

7 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

…d dev differentiation

- Previously, weekly/nightly builds were differentiated by flavor suffix in the
  package name (e.g., nvidia-dali-weekly-cuda*, nvidia-dali-nightly-cuda*).
  This change moves to a unified naming schema where:
  - Package names are consistent regardless of build type (nvidia-dali-cuda*)
  - Dev builds are differentiated by .dev{TIMESTAMP} suffix in version number
  - Stable and dev packages share the same package name, distinguished only by version
- Removes DALI_FLAVOR_MINUS variable from CMakeLists.txt files
- Updates package names to remove flavor suffix:
  - nvidia-dali-cuda* (was nvidia-dali-{flavor}-cuda*)
  - nvidia-dali-tf-plugin-cuda* (was nvidia-dali-tf-plugin-{flavor}-cuda*)
  - nvidia-dali-{plugin} (was nvidia-dali-{plugin}-{flavor})
- Standardizes dev version format: use .dev{TIMESTAMP} instead of .{TIMESTAMP}
- Updates install_requires in TF plugin to match new naming schema
- Updates installation documentation to explain new distribution method:
  - Documents new unified package naming with .dev{TIMESTAMP} version suffix
  - Explains use of --pre flag for installing dev builds
  - Adds legacy distribution section for pre-1.53 builds
  - Clarifies nightly vs weekly build schedule and availability

Signed-off-by: Janusz Lisiecki <[email protected]>
@JanuszL JanuszL force-pushed the new_dev_naming_schema branch from 2daff9a to 2bf93ca Compare December 4, 2025 08:26
@JanuszL
Copy link
Contributor Author

JanuszL commented Dec 4, 2025

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [39597446]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [39593786]: BUILD PASSED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [39597446]: BUILD PASSED

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