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

Add live updating for any render #382

Merged

Conversation

nathanrpage97
Copy link
Contributor

@nathanrpage97 nathanrpage97 commented Oct 13, 2020

Type of changes

  • Bug fix
  • New feature
  • Documentation / docstrings
  • Tests
  • Other

Checklist

  • I've run the latest black with default args on new code.
  • I've updated CHANGELOG.md and CONTRIBUTORS.md where appropriate.
  • I've added tests for new code.
  • I accept that @willmcgugan may be pedantic in the code review.

Description

I've been looking at ways to have live updating data table from rich. Whenever I used LiveRender it always faced any issues if something else was being logged.

So with that in mind I took what was there already from the Progress component and made a generalized Live Updating view. It allows for the component to be updated by the user with an update() function. It currently isn't threaded like Progress is, but could be expanded maybe with a LiveThreaded?

Limitations

If the render component is too large, then it will start too not fully clear any of the Renderable above the console view. I have messed around with creating a safe flag that will have it print "the terminal is too small" instead when this is true. I didn't want to add it here yet before getting feedback.

Showcase

rich_live_example

Some gist examples

@codecov
Copy link

codecov bot commented Oct 13, 2020

Codecov Report

Merging #382 (4236348) into master (d2fe147) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master     #382    +/-   ##
========================================
  Coverage   99.72%   99.72%            
========================================
  Files          54       55     +1     
  Lines        4654     4794   +140     
========================================
+ Hits         4641     4781   +140     
  Misses         13       13            
Flag Coverage Δ
unittests 99.72% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
rich/containers.py 100.00% <ø> (ø)
rich/__main__.py 100.00% <100.00%> (ø)
rich/bar.py 100.00% <100.00%> (ø)
rich/live.py 100.00% <100.00%> (ø)
rich/pretty.py 100.00% <100.00%> (ø)
rich/prompt.py 100.00% <100.00%> (ø)

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 f98cd31...4236348. Read the comment docs.

@nathanrpage97 nathanrpage97 marked this pull request as draft October 15, 2020 18:04
@willmcgugan
Copy link
Collaborator

I think this is a good idea! A number of people have asked for it.

It probably should have an option for thread-based auto refreshing so you could modify your data without having to explicitly refresh it. I'd also need typing, docstrings and tests, but I think you're on the right path. Let me know if you need any pointers.

@nathanrpage97
Copy link
Contributor Author

Great, I'm going to add in the threading next and then clean it up a bit. I think afterwards I can consolidate some of the design decisions this feature contains, that way it can be discussed upon what needs to be done. Then once those get through I can add testing and documentation.

@nathanrpage97 nathanrpage97 mentioned this pull request Oct 24, 2020
4 tasks
@nathanrpage97
Copy link
Contributor Author

@willmcgugan I'm currently facing an issue trying to check the output of these live Renders for testing in python 3.6. It seems to be related to handling of utf-8 encoding. Do you know of any workarounds?

@nathanrpage97
Copy link
Contributor Author

Note there are still issues with fluctuating sized displays. Will have to look into deeper.

@willmcgugan
Copy link
Collaborator

no worries. Let me know if you need any help with debugging. Will review soon (busy with the day job).

Mainly the issue comes between ensuring an ordering between new console
messages and the live render.

Not really sure how to add unit testing for this functionality.
@nathanrpage97
Copy link
Contributor Author

Ok so with some additional locking in the LiveRender and RefreshThread, it is now stable (and not deleting console log statements non-deterministically)

@yihong0618
Copy link

Can this pr solve #262 ?

@nathanrpage97
Copy link
Contributor Author

@yihong0618 No, this pr doesn't add any user input handling.

@yihong0618
Copy link

@yihong0618 No, this pr doesn't add any user input handling.

Thanks a lot.

@willmcgugan
Copy link
Collaborator

Sorry for the delay @nathanrpage97! Will look at this soon.

docs/source/live.rst Outdated Show resolved Hide resolved
rich/live.py Outdated Show resolved Hide resolved
@willmcgugan
Copy link
Collaborator

Excellent work @nathanrpage97 Testing now...

@willmcgugan
Copy link
Collaborator

willmcgugan commented Dec 2, 2020

Works great. I think my only request would be the ellipsis in the center.

You could render it like this, which would also solve the unlikely scenario of a terminal of 2 characters.

from rich.text import Text

yield from console.render(
    Text('...', overflow="crop", justify="center", end="")
)

@nathanrpage97
Copy link
Contributor Author

Alright the changes have been added in for table_movie_live.py -> table_movie.py and centered overflow ellipsis.

@willmcgugan
Copy link
Collaborator

Looks good. Thanks.

@willmcgugan willmcgugan merged commit b1612f9 into Textualize:master Dec 3, 2020
@nathanrpage97 nathanrpage97 deleted the feat/add-live-table-proposal branch December 4, 2020 04:55
@willmcgugan
Copy link
Collaborator

@nathanrpage97 Do you have a Twitter handle? I'd like you give you a shoutout when I release the next version of Rich.

@nathanrpage97
Copy link
Contributor Author

Awesome, I don’t use Twitter, so a GitHub plug instead?

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.

3 participants