Skip to content
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

Added cell description to tqdm #565

Merged
merged 5 commits into from Jan 22, 2022
Merged

Added cell description to tqdm #565

merged 5 commits into from Jan 22, 2022

Conversation

GiovanniCiampi
Copy link
Contributor

Added possibility to set description of cells that will be injected to the tqdm.
To add description to a specific cell the user must add a string with this formatting in the desired cell:
# papermill_description=description_of_cell
When the execution will reach that cell, the description of the tqdm will be changed from Executing x: to Executing description_of_cell.
If no cell contains the tag papermill_description=, the execution will be as before the implementation of this functionality.
If a cell containing the description tag is followed by cells that don't have the tag, the tqdm description is set to the last valid description.

I decided to implement this functionality because I think it is pretty useful with long notebooks that have very slow cells (i.e. data preparation and model training). An alternative can be to use logging, but it is way more verbose.
With this implementation you can simply add the 'Training' description to a cell and you know you are in the training process, or you can add 'Data_Preparation' and you know you are doing data preparation, moreover you see only the description of the current cell in the tqdm.
If a user doesn't like this functionality, he can simply ignore it.

I think it would be a good idea to add information about this feature in the documentation, but I would like to know first if this this is correctly implemented and perceived as a good addition from the authors/contributors.

@codecov
Copy link

codecov bot commented Jan 11, 2021

Codecov Report

Merging #565 (c79c6a5) into main (a318730) will increase coverage by 0.54%.
The diff coverage is 70.00%.

❗ Current head c79c6a5 differs from pull request most recent head b515364. Consider uploading reports for the commit b515364 to get more accurate results

@@            Coverage Diff             @@
##             main     #565      +/-   ##
==========================================
+ Coverage   91.65%   92.19%   +0.54%     
==========================================
  Files          17       16       -1     
  Lines        1558     1423     -135     
==========================================
- Hits         1428     1312     -116     
+ Misses        130      111      -19     

@GiovanniCiampi GiovanniCiampi marked this pull request as draft January 11, 2021 13:43
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@GiovanniCiampi GiovanniCiampi marked this pull request as ready for review September 17, 2021 08:47
Copy link
Member

@MSeal MSeal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the addition, well get this into the next release

@MSeal MSeal merged commit 59284a7 into nteract:main Jan 22, 2022
@MSeal
Copy link
Member

MSeal commented Jan 22, 2022

Overall the approach seems fine to me for an opt-in pattern. There's not a great programmatic pattern for fetching first comment in a cell outside of loading a full AST parser that's comment aware. This is simpler and less dependency requiring to achieve the same thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants