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

feat: add FileContents to read/watch files #62

Merged
merged 8 commits into from
Feb 10, 2023
Merged

feat: add FileContents to read/watch files #62

merged 8 commits into from
Feb 10, 2023

Conversation

manzt
Copy link
Owner

@manzt manzt commented Feb 9, 2023

Closes #61

This PR adds the anywidget._file_contents.FileContents object. FileContents watches for file changes in a provided path in a background thread and continuously emits changed signals any time the file changes, until the file is deleted (emits a deleted event). Connecting FileContents with MimeReprBuilder or AnyWidget allows for #60 to be leveraged during development. Currently this must be done manually, but we should aim to make the creation of FileContents hidden.

With traitlets

import anywidget
import traitlets
from anywidget._file_contents import FileContents

contents = FileContents("index.js")

class Counter(anywidget.AnyWidget):
    _esm = str(contents)
    value = traitlets.Int(0).tag(sync=True)

counter = Counter()
contents.changed.connect(lambda change: setattr(counter, "_esm", change)
counter

With MimeReprBuilder

import anywidget
from anywidget._file_contents import FileContents
from anywidget._descriptor_ import MimeReprBuilder

contents = FileContents("index.js")

@evented
@dataclasses
class Counter:
    _repr_mimebundle_ = MimeReprBuilder(_esm=str(contents)
    value = traitlets.Int(0).tag(sync=True)

counter = Counter()
contents.changed.connect(lambda change: counter._repr_mimebundle_._send_hmr_update(esm=change))
counter

@netlify
Copy link

netlify bot commented Feb 9, 2023

Deploy Preview for anywidget canceled.

Name Link
🔨 Latest commit 10d88d3
🔍 Latest deploy log https://app.netlify.com/sites/anywidget/deploys/63e597cfbda3600008da10a0

@codecov
Copy link

codecov bot commented Feb 9, 2023

Codecov Report

Merging #62 (10d88d3) into main (5b1c466) will decrease coverage by 0.07%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #62      +/-   ##
==========================================
- Coverage   98.34%   98.28%   -0.07%     
==========================================
  Files           6        7       +1     
  Lines         303      349      +46     
==========================================
+ Hits          298      343      +45     
- Misses          5        6       +1     
Impacted Files Coverage Δ
anywidget/_file_contents.py 100.00% <100.00%> (ø)
anywidget/_descriptor.py 98.93% <0.00%> (-0.54%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@manzt
Copy link
Owner Author

manzt commented Feb 9, 2023

Lol, not quite sure why the coverage is still messaged up. Anyways, I think this is probably ready for a review now. Like I said, still thinking about the right API to hide the FileContents for anywidget but I think that should probably be addressed in a subsequent PR.

@manzt manzt requested a review from tlambert03 February 9, 2023 22:12
Copy link
Collaborator

@tlambert03 tlambert03 left a comment

Choose a reason for hiding this comment

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

(still looking)

pyproject.toml Show resolved Hide resolved
tests/test_file_contents.py Outdated Show resolved Hide resolved
tests/test_file_contents.py Outdated Show resolved Hide resolved
anywidget/_file_contents.py Show resolved Hide resolved
anywidget/_file_contents.py Show resolved Hide resolved
Copy link
Collaborator

@tlambert03 tlambert03 left a comment

Choose a reason for hiding this comment

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

this is great. thanks for writing all the tests.

merge when ready (s'ok if you either want to revert my suggestion, or use the sleep approach)

@manzt
Copy link
Owner Author

manzt commented Feb 10, 2023

Thanks for the review! I tried the sleep but it failed in CI, so I'm reverting :) Learned a lot about python unit testing with this PR lol. Love it. thanks!

@manzt manzt merged commit 022d18e into main Feb 10, 2023
@manzt manzt deleted the file-contents branch February 10, 2023 01:06
@github-actions github-actions bot mentioned this pull request Feb 13, 2023
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.

2 participants