CLI improvements #32
CLI improvements #32
Conversation
Codecov Report
@@ Coverage Diff @@
## master #32 +/- ##
==========================================
- Coverage 40.80% 40.42% -0.38%
==========================================
Files 21 21
Lines 647 653 +6
Branches 48 48
==========================================
Hits 264 264
- Misses 383 389 +6
Continue to review full report at Codecov.
|
|
there is an issue apparently with isort <5.1.4 and setuptools v=49.2 more here |
|
it seems to be due to the fact that pylint doesn't support isort>5 |
stockroom/utils.py
Outdated
| console = Console() | ||
|
|
||
|
|
||
| def new_columns_table(splits_added: dict) -> Table: |
There was a problem hiding this comment.
Use a better (distinguishable) name so that readers won't get confused this column with hangar column
There was a problem hiding this comment.
wait what I meant is new_hangar_column_table. so column is hangar column is that how its suppose to be should I change it?
There was a problem hiding this comment.
Aah!! I was confused with table.add_column method. Maybe we could rename it as make_console_table or build_rich_table or tabularize_column_info or something similar?
stockroom/cli.py
Outdated
| stock_obj.commit(f"Data from {dataset_name} added through stock import") | ||
| stock_obj.close() | ||
| click.echo(f"The {dataset_name} dataset has been added to StockRoom") | ||
| click.echo(f"The {dataset_name} dataset has been added to StockRoom.") | ||
| console.print(new_columns_table(splits_added)) |
There was a problem hiding this comment.
can't we do
console = Console()
console.print(new_columns_table(splits_added))instead of using console from utils.py
There was a problem hiding this comment.
https://rich.readthedocs.io/en/latest/console.html#console-api
Here its mentioned that we should put this in a console.py file but I thought utils was better. If we want we can put all of the rich functionalities in this file or use initiate console every time. which would be nice?
|
Also, make sure your branch is rebased with the latest master |
stockroom/utils.py
Outdated
| column_names, num_samples = splits_added[split] | ||
| table.add_row(split + f" [{num_samples}]", ", ".join(column_names)) | ||
|
|
||
| return table |
There was a problem hiding this comment.
Continuing the conversation from here, maybe we shouldn't return from here at all. Just print it (of course, rename the function so that it is understood). What do you think? Because I don't see any reason why the object return from this function is useful anywhere else apart from just printing it to the output
There was a problem hiding this comment.
yes that a good point!
I'll change it to that 👍
|
one more thing |
Description
Why is this change required? What problem does it solve? Describe your changes in detail:
This incorporates the Rich library for better CLI UI
Rich will now be used through the CLI to create a more consistent theme.
If it fixes an open issue, please link to the issue here:
Screenshots (if appropriate):
Types of changes
What types of changes does your code introduce? Put an
xin all the boxes that apply: