-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Enhancement] Update digit_version function #865
Conversation
Codecov Report
@@ Coverage Diff @@
## master #865 +/- ##
==========================================
- Coverage 83.59% 83.44% -0.15%
==========================================
Files 176 176
Lines 14145 14032 -113
Branches 2364 2369 +5
==========================================
- Hits 11824 11709 -115
+ Misses 1713 1712 -1
- Partials 608 611 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
mmcv_maximum_version = '1.5.0' | ||
|
||
|
||
def digit_version(version_str: str, length: int = 4): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Importing is good enough, and do not need to copy the implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean importing from mmcv? I think the point of this PR is to avoid import this function from mmcv. Because digit_version from mmcv depends on mmcv version itself, like a circular dependency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually my thinking its the reverse. We will use the implementation of digit_version in mmcv, so that we will get automatic bug fixes.
Update
digit_version
function according to open-mmlab/mmcv#1185 and open-mmlab/mmcv#1235See also open-mmlab/mmpretrain#402