version: redesign output format for the dvc version command #3499 #4114

sahilbhosale63 · 2020-06-25T15:55:44Z

Fix #3499 redesign the output format for the dvc version command.

Redesigned the output format for the command: dvc version

…put-dvc Version: Redesign output format iterative#3499

skshetry · 2020-06-25T16:18:27Z

@sahilbhosale63, thanks for the PR. CI is failing. Did you run pre-commit hooks as mentioned here: https://dvc.org/doc/user-guide/contributing/core?

Also, it'd be good to see the outputs for different DVC installations/repos. Thanks.

skshetry · 2020-06-25T16:24:08Z

dvc/command/version.py

-        return ", ".join(supported_remotes)
+        if supported_remotes.__len__() == 9:
+            return "All remotes"
+        elif supported_remotes.__len__() == 1:


In Python, it's not good to use dunder methods directly(__len__). You can just use len().

dvc/command/version.py

Done some suggested fixes in code for the dvc command output format issue.

dvc/command/version.py

pmrowla

Changes look alright to me for the most part, but there's a couple general issues regarding the new format that I'm not sure about.

When running from a source installation instead of using any of the package/binary install methods (pip install -e ) we output

Build Info: Python 3.7.7 on Darwin-19.5.0-x86_64-i386-64bit installed via None

It might be better for us to just drop the installed via ... part entirely in this case? When I first read None here, it made me think this was a bug in how we detect installation method, rather than thinking it meant "None as in not installed from a binary package".

This was a bit clearer in the old output when we had

Binary: False
Package: None

Also in the new output we do:

Directory cache: apfs on /dev/disk1s1
...
Workspace with apfs on /dev/disk1s1

and it looks a bit strange to have inconsistent formatting between both of these lines (in one line we use a colon and in the other line we use with)

skshetry · 2020-06-29T07:40:29Z

dvc/command/version.py

+        if len(supported_remotes) == len(TREES):
+            return "All remotes"
+        elif len(supported_remotes) == 1:
+            return supported_remotes
+        else:
+            return ", ".join(supported_remotes)


@sahilbhosale63, the plan was to make Supports use both remotes and cache types. But, as we didnot go through that, I think it's okay to not go "All remotes" route (i.e what was before with comma separated list of remotes). But, it's your call.

skshetry · 2020-06-29T07:44:21Z

@sahilbhosale63, @pmrowla, maybe we could do something like this:

Build Info: Python 3.7.7 on Darwin-19.5.0-x86_64-i386-64bit (pip)
Build Info: Python 3.7.7 on Darwin-19.5.0-x86_64-i386-64bit (binary)

Also, it would be good to rename Build Info to Platform which is what it is.

Or, could add it to the version: DVC version: 1.0.0 (pip)?

pmrowla · 2020-06-29T07:52:07Z

Or, could add it to the version: DVC version: 1.0.0 (pip)?

This might work best just because the platform line is already going to be pretty long in a lot of cases (even without the installation method)

sahilbhosale63 · 2020-07-21T04:00:41Z

@skshetry can you please review this patch?

skshetry

I am really sorry for taking it so long to review, looks good in general, just a few minor issues.

skshetry · 2020-07-21T04:24:11Z

dvc/command/version.py

-            f"Binary: {is_binary()}",
-            f"Package: {PKG}",
-            f"Supported remotes: {self.get_supported_remotes()}",
+            f"DVC version: {__version__} ({PKG})",


I get:

DVC version: 1.0.1+412f39 (None)

PKG can be None too. So, let's leave that out if it's None.

skshetry · 2020-07-21T04:24:48Z

dvc/command/version.py

@@ -46,13 +45,11 @@ def run(self):
            # `dvc config cache.shared group`.
            if os.path.exists(repo.cache.local.cache_dir):
                info.append(
-                    "Cache: {}".format(self.get_linktype_support_info(repo))
+                    "Cache types: " + self.get_linktype_support_info(repo)


Let's use {} formated string as above with just the addtion of Cache types:.

dvc/command/version.py

skshetry · 2020-07-21T04:28:20Z

dvc/command/version.py

@@ -139,7 +144,7 @@ def _get_dvc_repo_info(repo):
    if repo.root_dir != repo.scm.root_dir:
        return "dvc (subdir), git"

-    return "dvc, git"
+    return "dvc + git"


We need to make this uniform, either we should make it all commas or all plus-es. Could just use comma for now.

Same for cache types which seems to use / right now.

+1 for commas (since / is a path separator)
+1 to check other occurrences

dvc/command/version.py

sahilbhosale63 · 2020-07-21T15:29:21Z

@skshetry @pmrowla @jorgeorpinel Does this looks good or should I make some changes?

jorgeorpinel · 2020-07-22T03:37:46Z

I just noticed this PR, thanks for the mention.

Supports: All remotes

How about

Supported remotes: {List them always} — actually I think that's how it is now so maybe this doesn't need a change...

Also:

Cache types -> Supported cache types ?
Workspace Directory - lower case directory
Cache Directory - same ^. And put after Workspace dir?
Repo: -> Repo control: ?

Also keep in mind this may need updates to https://dvc.org/doc/command-reference/version or other docs after merge. Thanks

shcheklein · 2020-07-22T03:41:41Z

@sahilbhosale63 why do we need (----------------) separator?

How does the output looks like when no cache dir is available?
I don't see the install method ... is it None on your screenshot? should we use some human readable name?

sahilbhosale63 · 2020-07-22T03:59:13Z

@sahilbhosale63 why do we need (----------------) separator?

How does the output looks like when no cache dir is available?
I don't see the install method ... is it None on your screenshot? should we use some human readable name?

@shcheklein When there is no cache dir it gives a warning. We have dicussed about this previously here #3499 (comment)

The PKG is (None) because here in this case we have not installed dvc using pip or any other package manager.

Yes, we need that seperator as decided here #3499

sahilbhosale63 · 2020-07-22T08:40:17Z

@jorgeorpinel Added #3499 issue number to the PR description with the Fix keyword.

sahilbhosale63 · 2020-07-23T02:54:21Z

The Travis build test failures are unrelated to the changes which I have made.

dvc/command/version.py

skshetry

LGTM, just not sure about All remotes, but fine for now.

pmrowla

LGTM

Co-authored-by: Saugat Pachhai <[email protected]>

efiop · 2020-07-28T17:29:32Z

Thank you @sahilbhosale63 ! 🙏

jorgeorpinel · 2020-07-29T03:18:42Z

Created iterative/dvc.org/issues/1635 to update the docs.

sahilbhosale63 added 2 commits June 25, 2020 19:54

Version: Redesign output format iterative#3499

365713e

Redesigned the output format for the command: dvc version

Merge pull request #1 from sahilbhosale63/sahilbhosale63-reformat-out…

361f562

…put-dvc Version: Redesign output format iterative#3499

efiop requested a review from skshetry June 25, 2020 16:00

skshetry reviewed Jun 25, 2020

View reviewed changes

dvc/command/version.py Outdated Show resolved Hide resolved

skshetry reviewed Jun 25, 2020

View reviewed changes

dvc/command/version.py Outdated Show resolved Hide resolved

sahilbhosale63 added 3 commits June 26, 2020 22:26

version: reformat output format iterative#3499

d68ddb2

Done some suggested fixes in code for the dvc command output format issue.

version: redesign output format iterative#3499

96add40

version: redesign output format iterative#3499

e9cb620

pmrowla suggested changes Jun 27, 2020

View reviewed changes

dvc/command/version.py Outdated Show resolved Hide resolved

version: redesign output format for dvc command iterative#3499

7f66772

sahilbhosale63 requested a review from pmrowla June 28, 2020 11:24

pmrowla reviewed Jun 29, 2020

View reviewed changes

dvc/command/version.py Outdated Show resolved Hide resolved

pmrowla reviewed Jun 29, 2020

View reviewed changes

dvc/command/version.py Outdated Show resolved Hide resolved

pmrowla suggested changes Jun 29, 2020

View reviewed changes

skshetry reviewed Jun 29, 2020

View reviewed changes

weekly-digest bot mentioned this pull request Jul 5, 2020

Weekly Digest (28 June, 2020 - 5 July, 2020) #4168

Closed

sahilbhosale63 added 2 commits July 11, 2020 13:02

version: redesign output format iterative#3499

6064659

version: redesign output format iterative#3499

412f39d

weekly-digest bot mentioned this pull request Jul 12, 2020

Weekly Digest (5 July, 2020 - 12 July, 2020) #4195

Closed

efiop requested a review from skshetry July 17, 2020 02:38

weekly-digest bot mentioned this pull request Jul 19, 2020

Weekly Digest (12 July, 2020 - 19 July, 2020) #4238

Closed

skshetry reviewed Jul 21, 2020

View reviewed changes

dvc/command/version.py Show resolved Hide resolved

This comment has been minimized.

Sign in to view

sahilbhosale63 added 2 commits July 23, 2020 07:09

Redesign output format for the dvc version command iterative#3499

4e55522

Redesign output format for the dvc version command iterative#3499

753efe7

weekly-digest bot mentioned this pull request Jul 26, 2020

Weekly Digest (19 July, 2020 - 26 July, 2020) #4285

Closed

skshetry reviewed Jul 28, 2020

View reviewed changes

dvc/command/version.py Outdated Show resolved Hide resolved

skshetry reviewed Jul 28, 2020

View reviewed changes

skshetry requested review from efiop, pared and pmrowla July 28, 2020 11:52

pmrowla approved these changes Jul 28, 2020

View reviewed changes

pared approved these changes Jul 28, 2020

View reviewed changes

Update dvc/command/version.py

999c9e1

Co-authored-by: Saugat Pachhai <[email protected]>

efiop merged commit f4595a0 into iterative:master Jul 28, 2020

jorgeorpinel mentioned this pull request Jul 29, 2020

cmd: update version ref. iterative/dvc.org#1635

Closed

weekly-digest bot mentioned this pull request Aug 2, 2020

Weekly Digest (26 July, 2020 - 2 August, 2020) #4315

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

version: redesign output format for the dvc version command #3499 #4114

version: redesign output format for the dvc version command #3499 #4114

sahilbhosale63 commented Jun 25, 2020 •

edited

Loading

skshetry commented Jun 25, 2020 •

edited

Loading

skshetry Jun 25, 2020

pmrowla left a comment

skshetry Jun 29, 2020 •

edited

Loading

skshetry commented Jun 29, 2020

pmrowla commented Jun 29, 2020

sahilbhosale63 commented Jul 21, 2020

skshetry left a comment

skshetry Jul 21, 2020

skshetry Jul 21, 2020

skshetry Jul 21, 2020

jorgeorpinel Jul 22, 2020

shcheklein Jul 22, 2020

sahilbhosale63 commented Jul 21, 2020 •

edited

Loading

jorgeorpinel commented Jul 22, 2020

shcheklein commented Jul 22, 2020

This comment has been minimized.

sahilbhosale63 commented Jul 22, 2020 •

edited

Loading

This comment has been minimized.

sahilbhosale63 commented Jul 22, 2020 •

edited by jorgeorpinel

Loading

sahilbhosale63 commented Jul 23, 2020 •

edited

Loading

skshetry left a comment

pmrowla left a comment

efiop commented Jul 28, 2020

jorgeorpinel commented Jul 29, 2020

version: redesign output format for the dvc version command #3499 #4114

version: redesign output format for the dvc version command #3499 #4114

Conversation

sahilbhosale63 commented Jun 25, 2020 • edited Loading

skshetry commented Jun 25, 2020 • edited Loading

skshetry Jun 25, 2020

Choose a reason for hiding this comment

pmrowla left a comment

Choose a reason for hiding this comment

skshetry Jun 29, 2020 • edited Loading

Choose a reason for hiding this comment

skshetry commented Jun 29, 2020

pmrowla commented Jun 29, 2020

sahilbhosale63 commented Jul 21, 2020

skshetry left a comment

Choose a reason for hiding this comment

skshetry Jul 21, 2020

Choose a reason for hiding this comment

skshetry Jul 21, 2020

Choose a reason for hiding this comment

skshetry Jul 21, 2020

Choose a reason for hiding this comment

jorgeorpinel Jul 22, 2020

Choose a reason for hiding this comment

shcheklein Jul 22, 2020

Choose a reason for hiding this comment

sahilbhosale63 commented Jul 21, 2020 • edited Loading

jorgeorpinel commented Jul 22, 2020

shcheklein commented Jul 22, 2020

This comment has been minimized.

sahilbhosale63 commented Jul 22, 2020 • edited Loading

This comment has been minimized.

sahilbhosale63 commented Jul 22, 2020 • edited by jorgeorpinel Loading

sahilbhosale63 commented Jul 23, 2020 • edited Loading

skshetry left a comment

Choose a reason for hiding this comment

pmrowla left a comment

Choose a reason for hiding this comment

efiop commented Jul 28, 2020

jorgeorpinel commented Jul 29, 2020

sahilbhosale63 commented Jun 25, 2020 •

edited

Loading

skshetry commented Jun 25, 2020 •

edited

Loading

skshetry Jun 29, 2020 •

edited

Loading

sahilbhosale63 commented Jul 21, 2020 •

edited

Loading

sahilbhosale63 commented Jul 22, 2020 •

edited

Loading

sahilbhosale63 commented Jul 22, 2020 •

edited by jorgeorpinel

Loading

sahilbhosale63 commented Jul 23, 2020 •

edited

Loading