printing dataset_id and important filenames/values in blue

[MalteEbner]

closes #332
closes https://github.com/lightly-ai/lightly-core/issues/352

Description

• My change is breaking
• The lightly-upload command now also prints the dataset_id if the new_dataset_name was specified.
• Many important filenames/values are now printed in blue.
• The lightly-magic command now prints e.g. ########## Starting to embed your dataset. before each of its three steps
• More unittests for lightly-download to also cover new printings in blue
• added helper to print missing cli api arguments as yellow warnings, but without text

lightly-magic: (cut off parts of the start)

lightly-download:

Tests

• My change is covered by existing tests
• My change needs new tests
• I have added/adapted tests accordingly.
• I have manually tested the change.

If applicable, describe the manual test procedure, e.g:

pip uninstall lightly
export BRANCH_NAME="332-Return-the-dataset_id-when-creating-a-new-dataset-me"
pip install "git+https://github.com/lightly-ai/lightly.git@$BRANCH_NAME"
lightly-magic new_dataset_name='new_dataset_name_xyz' token=MY_TOKEN trainer.max_epochs=1 input_dir='my_input_dir'
lightly-download token= MY_TOKEN dataset_id='was_in_blue_in_CLI_print' tag_name='initial-tag' input_dir='my_input_dir' output_dir='my_output_dir'

Documentation

• I have added docstrings to all changed/added public functions/methods.
• My change requires a change to the documentation ( .rst files).
• I have updated the documentation accordingly.
• The autodocs update the documentation accordingly.`

[japrescott]

looks good with my limited knowledge :)

Questions;

• Can we use a custom blue (like our lightly blue? #2BA3D1 :)) )
• Can we color the upload/progress bar different from the rest of the information? e.g white?

Suggestions;

• Maybe we would also want to show warnings/errors in an orange/red color to make it more eye-popping when it occurs?

[japrescott]

since every test has this line as well, it would call this multiple times, no?

[MalteEbner]

Yes, it is called once for every test. It runs against the mocked server.
We could put those two lines into an extra function which is called directly with the cli_string. However, this would only save one line per test, thus it is not worth it in my eyes.

[japrescott]

nope, I just realised its called twice thats all. If that is intended behaviour 1) to set it up and 2) to then call it correctly, then all is good

[MalteEbner]

• Can we use a custom blue (like our lightly blue? #2BA3D1 :)) )
• Maybe we would also want to show warnings/errors in an orange/red color to make it more eye-popping when it occurs?

We can also have it blink, change the background and font and even more:
https://stackoverflow.com/questions/4842424/list-of-ansi-color-escape-sequences

However, I would put those ideas into a new issue, as they need a discussion about which design languages to follow. We should not overwhelm our users with a rainbow of colours.

[MalteEbner]

• Can we color the upload/progress bar different from the rest of the information? e.g white?

We use tqdm for the bar, so this is possible with some tricks: tqdm/tqdm#450
However, I would not do it, as other common frameworks (pytorch, tensorflow) don't do it either and consider it as distracting.

[IgorSusmelj]

Looks great!

[japrescott]

I thought you wanted an open discussion of how we want to handle it, since you also added the ########## "styling" instead of just colouring info text differently.

Which was requested in https://github.com/lightly-ai/lightly-core/issues/352 as far as I understood "clear separation between sections"

However, I had a closer look at our warnings and found that they were inconsistent: Some were in the warnings.warn() format, others just printed. I aligned that by introducing a helper function for printing as warnings, but in yellow and without giving the exact line of the warning.

[philippmwirth]

Looks good! The only thing I don't like is that there seems to be no linebreak before printing the dataset id.

Other than that I would love it if you could add one print statement (or two) for the embedding upload.

[codecov]

Codecov Report

Merging #356 (6906efd) into develop (f58144f) will increase coverage by 0.81%.
The diff coverage is 97.67%.

@@             Coverage Diff             @@
##           develop     #356      +/-   ##
===========================================
+ Coverage    84.21%   85.03%   +0.81%     
===========================================
  Files           70       70              
  Lines         2408     2432      +24     
===========================================
+ Hits          2028     2068      +40     
+ Misses         380      364      -16     

Impacted Files	Coverage Δ
lightly/cli/_helpers.py	80.28% <90.00%> (+1.59%)	⬆️
lightly/cli/download_cli.py	94.73% <100.00%> (+18.46%)	⬆️
lightly/cli/embed_cli.py	90.32% <100.00%> (+0.15%)	⬆️
lightly/cli/lightly_cli.py	96.29% <100.00%> (+0.64%)	⬆️
lightly/cli/train_cli.py	85.71% <100.00%> (+0.18%)	⬆️
lightly/cli/upload_cli.py	94.82% <100.00%> (+1.07%)	⬆️
lightly/data/dataset.py	94.38% <0.00%> (+6.74%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f58144f...6906efd. Read the comment docs.

[MalteEbner]

Looks good! The only thing I don't like is that there seems to be no linebreak before printing the dataset id.

This is due to tqdm, which creates those artifacts due to race conditions of the multiple workers. I added one line to solve it:

lightly/lightly/cli/upload_cli.py

Line 70 in a03a876

print(f"Finished the upload of the dataset.")

Other than that I would love it if you could add one print statement (or two) for the embedding upload.

I added those statements:

lightly/lightly/cli/upload_cli.py

Lines 73 to 77 in 485163a

	print("Starting upload of embeddings.")
	api_workflow_client.upload_embeddings(
	path_to_embeddings_csv=path_to_embeddings, name=name
	)
	print("Finished upload of embeddings.")