-
Notifications
You must be signed in to change notification settings - Fork 177
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
Script consistency #390
Script consistency #390
Conversation
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 pretty good, thanks Andrew.
There's just a few style issues that I'd like you to fix up before we merge this.
Cheers
datacube/scripts/product.py
Outdated
""" | ||
List products that are defined in the generic index. | ||
""" | ||
LIST_OUTPUT_WRITERS[f]((build_product_list(dc.index) |
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.
The double parentheses here are unnecessary and make it look more confusing than needed, this line can be simplified to:
LIST_OUTPUT_WRITERS[f](build_product_list(dc.index))
datacube/scripts/user.py
Outdated
writer.writeheader() | ||
|
||
#for role, user, description in index: | ||
#click.echo('{0:6}\t{1:15}\t{2}'.format(role, user, description if description else '')) |
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.
Please remove any commented out code. It ends up being really confusing for anyone reading the code in the future.
datacube/scripts/product.py
Outdated
|
||
# We can't control how many ancestors this dumper API uses. | ||
# pylint: disable=too-many-ancestors | ||
class OrderedDumper(yaml.SafeDumper): |
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.
This class should be moved out from inside these methods, and shared between the different command line scripts. It needs a more descriptive name, and should include all the features that are required in different places, ie. Safe, and able to represent OrderedDicts
, Dates and Ranges
Either create a common.py
file inside the scripts/
directory. Or put it inside datacube/utils/
.
It's safe for the single
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 Andrew
be5976e
to
2f56bfd
Compare
Reason for this pull request
Add options for formatted output for product list/show and user list. This includes yaml as default, as well as csv and plain table formats. Formatting is only applied to the outputs as appropriate.
...
Proposed changes
Add yaml, csv, table format to product/list.
Add yaml, csv, json format to product/show
Add yaml, csv format to user/list
Resolve minor formatting errors in api/core.py and utils/nbartprepare.py
Closes CLI list/show subcommands should be consistent between types #206
Tests passing
Fully documented, including
docs/about/whats_new.rst
for all changes