-
Notifications
You must be signed in to change notification settings - Fork 1.3k
experiments: support column ordering & sorting in dvc exp show
#4503
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
Conversation
|
|
||
| if include: | ||
| ret = set() | ||
| ret = OrderedDict() |
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.
Use OrderedDict() with None values to get set-like behavior with preserved ordering
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.
Or, use a dict?
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.
I thought the preserved key ordering for regular dict technically isn't guaranteed before 3.7 (it's a cpython implementation behavior in 3.6 that was added to the formal spec in 3.7), so I thought it would be preferred to just use OrderedDict?
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.
yes, you are right that it is only on >3.6. My point was that, as CPython and Python are closely tied, no one is going to change that behavior in 3.6, so should be safe to just use dict for convenience. Saying that I have no issues with OrderedDict, it was just a suggestion.
skshetry
left a comment
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.
Looks good. Just one suggestion.
|
|
||
| if include: | ||
| ret = set() | ||
| ret = OrderedDict() |
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.
Or, use a dict?
β I have followed the Contributing to DVC checklist.
π If this PR requires documentation updates, I have created a separate PR (or issue, at least) in dvc.org and linked it here.
Thank you for the contribution - we'll try to review it as soon as possible. π
Will close #4346.
--include-metrics/--include-params, columns will be displayed left-to-right in the order specified by the--includeoption.--sort-by <column_name>and--sort-order asc|desccan be used to sort related experiments on the specified metric or param column.--sort-orderdefaults to ascending