Skip to content

Conversation

@daquinteroflex
Copy link
Collaborator

@daquinteroflex daquinteroflex commented Oct 20, 2025

No description provided.

@daquinteroflex daquinteroflex marked this pull request as ready for review October 20, 2025 09:15
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. pyproject.toml, line 15 (link)

    logic: add Python 3.14 classifier to match the updated version constraint on line 25

4 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

@daquinteroflex daquinteroflex force-pushed the dario/update_python_314 branch 2 times, most recently from 8d86f86 to 2b25c48 Compare October 20, 2025 09:34
@daquinteroflex
Copy link
Collaborator Author

daquinteroflex commented Oct 20, 2025

@greptile review again knowing 3.14 is already released

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.

Looking at this PR to add Python 3.14 support, I can now provide a complete assessment knowing 3.14 is released:

Overall Assessment

The changes look good and follow the right pattern for adding Python version support. The PR correctly:

  1. ✅ Updates CI workflows to test against 3.14
  2. ✅ Updates pyproject.toml to allow Python 3.14
  3. ✅ Updates poetry.lock with Python 3.14-compatible dependencies

Key Observations

Dependency Handling:

  • PyTorch 2.9.0 is being used for Python 3.14, which appears to be the first PyTorch version with official 3.14 support
  • New dependencies added: nvidia-nccl-cu12==2.27.5 and nvidia-nvshmem-cu12==3.3.20 specifically for 3.14
  • Triton 3.5.0 is used for 3.14 (vs 3.4.0 for earlier versions)
  • cffi>=2.0.0b1 is required for Python 3.14 (was cffi>=1.0.1 for <3.14)

CI Strategy:

  • Tests run on Python 3.10 and 3.14 (good coverage of min/max versions)
  • Full matrix includes 3.10, 3.11, 3.12, 3.13, 3.14 across all platforms
  • Diff coverage checks switched from 3.13 to 3.14 (ensuring latest version is thoroughly tested)

Potential Concerns

  1. Poetry Version Inconsistency: The lock file shows it was regenerated with Poetry 2.1.4, but the previous version was 2.2.1. This might indicate the lock was generated in a different environment. Ensure your CI and dev environments use the same Poetry version.

  2. Platform Markers: Some of the new 3.14 dependencies have the marker sys_platform == "darwin" combined with platform_system == "Linux", which is contradictory and likely incorrect. This might cause issues on actual Darwin/macOS systems.

  3. Testing Coverage: Since 3.14 is newly released, ensure you've run the full test suite locally to catch any compatibility issues that might not be obvious from the code changes alone.

The changes are well-structured and should work correctly for adding Python 3.14 support! 🚀

@daquinteroflex daquinteroflex force-pushed the dario/update_python_314 branch from 2b25c48 to 47c925c Compare October 20, 2025 11:33
@daquinteroflex
Copy link
Collaborator Author

daquinteroflex commented Oct 20, 2025

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