iterative / dvc

🦉 Data Versioning and ML Experiments
https://dvc.org
Apache License 2.0
13.87k stars 1.19k forks source link

Consistent cmd option order? #3642

Closed jorgeorpinel closed 11 months ago

jorgeorpinel commented 4 years ago

Extracted from https://github.com/iterative/dvc/issues/3608#issuecomment-615008799

To match docs, where options have been ordered manually from more to less important (at least that's the idea).

Example difference:

λ dvc commit -h
usage: dvc commit [-h] [-q | -v] [-f] [-d] [-R] [targets [targets ...]]
...

optional arguments:
  -h, --help            show this help message and exit
  -q, --quiet           Be quiet.
  -v, --verbose         Be verbose.
  -f, --force      Commit even if hash value for dependencies/outputs changed.
  -d, --with-deps  Commit all dependencies of the specified target.
  -R, --recursive  Commit cache for subdirectories of the specified directory.

(DVC 0.93.0)

vs. https://dvc.org/doc/command-reference/commit#options:

image

jorgeorpinel commented 4 years ago

I'm wondering whether -h -q -v can be moved to the bottom of all cmd -h outputs as a first step..

efiop commented 4 years ago

@jorgeorpinel They probably could be moved with some hack or something, but I would urge you to consider if this is really worth it.

efiop commented 4 years ago

@jorgeorpinel I guess worth elaborating that moving -h/-v would require a non-trivial hack for argparse, which is probably too much effort for this kind of change. Not as easy as moving few lines around, unfortunately :slightly_frowning_face:

jorgeorpinel commented 4 years ago

Yes, -h -q -v def. don't seem worth trying to move. I'm not sure about the other ones though, let's say you have 20 lines in your terminal vertically (most people will have more but still) you use -h on a big command like run and scroll back to the top of the output. Shouldn't the more commonly used options be shown first? It could potentially save time and make the experience a little better.

It's probably marginal so I'm not obsessing over this or anything. We can just leave it open and if we never get to it just close it at some point. Or close it if you feel strongly against this, np on my side 🙂

efiop commented 4 years ago

@jorgeorpinel Those are really good points! If we look at it that way, maybe it is worth hacking it at some point. Also, interesting to note that things like awscli don't even use argparse and use their own implementation to be able to polish the experience. I don't have a strong opinion.

efiop commented 4 years ago

@jorgeorpinel how about we just sort in alphabetical order to start with? The "commonly used" approach is a nice extension, but it will require collecting some stats and is only really relevant for long help. So, for example, for dvc -h, where we have all of the commands and where indeed it might be nice to have commonly-used ones more visible.

jorgeorpinel commented 4 years ago

Sure, that's a reasonable approach which simplifies maintaining core and docs @efiop.

only really relevant for long help

Also agree, although long help probably covers all the most important commands and like 80% of the help text overall so not sure that if we ever implement that it's worth discriminating.

shcheklein commented 4 years ago

how about we just sort in alphabetical order to start with?

tbh, I would prefer to keep them in some reasonable order, at least on the docs side (important to less important, or in some cases there should be blocks that cluster related things, e.g. --all-commits, --all-branches.). We def can see auxiliary options like (-q/h/v) and most important options specific to the command.

sorting alphabetical sounds like random to me

efiop commented 4 years ago

@shcheklein Sure, we could leverage add_argument_group and separate those into groups that we like. Within those it would still be nice to just sort things alphabetically, because otherwise we'll need to keep the ordering in mind when writing code, which is a chore that we would like to avoid.

So I would first create a formatter_class for alphabetical sort and then would create separate groups as the next step, because it will require some time to go through every command.

EDIT: e.g.

Git control
    --all-tags ...
    --all-branches ...
    --all-commits ...
Debug options
    --verbose
    ...
naspeh commented 4 years ago

Few words from me.

I guess worth elaborating that moving -h/-v would require a non-trivial hack for argparse

Moving -q -v -h options can be achieved with an explicit solution (no hack needed). I suggested two approaches in #4681 and #4695. I can think of a third and fourth solution with different refactorings, but I don't want to work further on this as I see there is no decision on what this ticket requires: alphabetical order vs moving -q -v -h args to the bottom. I think the good first issue label could be removed until a clear decision on that.


Automation

To have consistent order in docs and real dvc, I think we could consider another approach, we could try to generate help text for commands and include the output into docs (maybe with some additional transformations to get a greater look), it could simplify maintenance for this task.

jorgeorpinel commented 4 years ago

To be clear I was in favor of alphabetical order as an alternative to no order (rather the order in which options are added to the code, as is now). We still want to maintain an order of (assumed) relevance in docs, indeed.

@efiop I'm not sure about the groups for sorting by relevance. If you see docs now, all kinds of options are put first, simply because we assume they will be used more often/cover more user needs. That said if you have a comprehensive proposal of what the options groups are, we could take it from there. But it feels like a separate discussion.

@naspeh thanks for the contributions. This is a bit more than moving -qvh to the bottom. If you know of a way to easily specify the arg order in Python (without having to rewrite the whole code base and worry about it on every arg/param change) that would be great. p.s. automatically generating docs for args is also a separate issue.

Have you guys considered this? https://stackoverflow.com/a/9028031/761963 — could it be coupled with a config file where we can maintain the arg order independently?