update requirements#51
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the version constraints for peft and transformers in requirements.txt. The review feedback suggests adding packaging as a direct dependency since it is used in the codebase and recommends restricting the transformers version to <5.0.0 to avoid potential breaking changes in future major releases.
| megatron-core>=0.12 | ||
| modelscope | ||
| peft>=0.11,<0.19 | ||
| peft>=0.11,<0.20 |
There was a problem hiding this comment.
The packaging library is imported in src/mcore_bridge/patcher.py and src/mcore_bridge/tuners/lora.py but is not listed as a direct dependency in requirements.txt. It is best practice to explicitly declare all direct dependencies to ensure environment consistency and avoid potential ImportError if transitive dependencies change.
peft>=0.11,<0.20
packaging
| safetensors | ||
| tqdm | ||
| transformers>=4.33,<5.6.0 | ||
| transformers>=4.33,<5.7.0 |
There was a problem hiding this comment.
The upper bound constraint <5.7.0 for transformers is unconventional. Since transformers is currently in the 4.x series, a major version release (5.0.0) is expected to contain breaking changes. Given that this project performs extensive monkey-patching on internal APIs, it is safer to restrict the dependency to the current major version (e.g., <5.0.0) unless compatibility with 5.x has been explicitly verified.
transformers>=4.33,<5.0.0
1 participant
No description provided.