-
Notifications
You must be signed in to change notification settings - Fork 16
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
Enhance sorting #162
Enhance sorting #162
Conversation
gto/base.py
Outdated
if sort == VersionSort.SemVer: | ||
# sorting SemVer versions in a right way | ||
sorted_versions = sorted( | ||
(v for v in versions if v.is_registered), |
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.
@aguschin We don't store is_registered or discovered tag with us. While using the method with dict, v.is_registered
may throw AttributeError here.
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.
Ok, I see your example repo doesn't include these cases, so I think you didn't implement support for not is_registered
versions.
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.
@amritghimire, I've made it infer whether version is SemVer or just a hexsha. Will it work now?
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.
Thanks, it works now.
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.
Do you need something else to be implemented now? Otherwise I'll go ahead and create new release.
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.
That is all @aguschin . Thanks for continuous updates to the package.
@amritghimire, please check out this PR. You need
def sort_versions
inbase.py
Note, that BaseVersion in GTO has
is_registered
anddiscovered
atrrs, which I use for sorting also. Not sure you have them already.is_registered == True
indicates that it's an explicitly registered SemVer, e.g.v1.2.3
.is_registered == False
indicates that it wasn't explicitly registered, but model in this hexsha was promoted to some stage (e.g. withmodel#prod#1
git tag). In this case GTO assumes it's a version, though not explicitly registered. For the version name, GTO uses commit hash.Considering #132
is_registered == False
maybe deprecated. But we need a decision there first.cc @shcheklein: #132 is something I can use your help with.