Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 33 additions & 3 deletions hotload.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,21 @@ def _file_changed(f):
def _all_file_changes(filepaths):
return {path: _file_changed(path) for path in filepaths}

def _changed_modules(new_changed, last_changed):
list = []
Copy link
Copy Markdown
Owner

@teodorlu teodorlu Jul 3, 2022

Choose a reason for hiding this comment

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

list shadows the list class.

Perhaps call it changed_modules or something else?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

👍

if last_changed is None:
return list
modules = {}
for module in sys.modules.copy().values():
file = getattr(module, '__file__', None)
if file is not None:
modules[file] = module
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

(Could this module loop be a performance issue? An alternative could be to store the modules similar to watchfiles)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This far I haven't really explored possible performance hits. Polling is bad (in general).

Why?

I simply haven't yet needed hotload when working on codebases that are so big that performance becomes an issue.

for path in new_changed:
if new_changed[path] != last_changed[path]:
module = modules.get(path, None)
if module is not None:
list.append(module)
return list

def listfiles(folder, ext=""):
fs = list()
Expand Down Expand Up @@ -135,6 +150,13 @@ class ClearTerminal(Runnable):
def run(self):
os.system("cls" if os.name == "nt" else "clear")

class ReloadModules(Runnable):
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

perhaps not really a Runnable when run requires an additional argument?

Copy link
Copy Markdown
Owner

@teodorlu teodorlu Jul 3, 2022

Choose a reason for hiding this comment

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

I'm open to allowing run accept (*args, **kwargs). In that case, we could always provide a list of modules.

step.run(changed_modules=changed_modules)

Though if the list of modules is passed to ReloadModules on __init__, perhaps it doesn't need any special run() treatment?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Edit - I see that you're only reloading modules that have changed. In that case, we'd need to pass the list of changed modules.

def __init__(self, module):
self.main_module = module
def run(self, modules):
for module in modules:
if module != self.main_module:
_reload_module(module)

def hotload(watch, steps, waittime_ms=1.0 / 144):
"""Hotload that code!"""
Expand All @@ -148,18 +170,25 @@ def hotload(watch, steps, waittime_ms=1.0 / 144):
# Take note of when files were last changed before we start reloading
last_changed = None

is_module_reloader = lambda step: type(step).__name__ == "ReloadModules"
do_reload_modules = bool(list(filter(is_module_reloader, steps)))

# Begin the loop! Each Runner is responsible for handling its own exceptions.
while True:
new_changed = _all_file_changes(watchfiles)
if last_changed == new_changed:
time.sleep(waittime_ms)
else:
reload_begin_ms = time.time() * 1000
changed_modules = _changed_modules(new_changed, last_changed) if do_reload_modules else []
last_changed = new_changed
try:
for step in steps:
try:
step.run()
if is_module_reloader(step):
step.run(changed_modules)
else:
step.run()
except KeyboardInterrupt:
raise
except:
Expand Down Expand Up @@ -210,7 +239,7 @@ def main():
print()
print(" ls *py | hotload hello.py")

watchfiles = [f.strip() for f in sys.stdin.readlines()]
watchfiles = [os.path.normpath(os.path.join(os.getcwd(), f.strip())) for f in sys.stdin.readlines()]

if not watchfiles:
print("Error: no watch files specified.")
Expand All @@ -227,7 +256,7 @@ def main():

conf = {
"watch": [watchfiles],
"steps": [ClearTerminal(), reloaded_module],
"steps": [ClearTerminal(), ReloadModules(reloaded_module.module), reloaded_module],
Copy link
Copy Markdown
Owner

@teodorlu teodorlu Jul 3, 2022

Choose a reason for hiding this comment

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

ReloadModules(reloaded_module.module) reads kind of funny to me. Lots of "module", but not necessarily easy to understand what's going on.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes, reading the implementation is necessary to understand this line.

Naming the argument could help: ReloadModules(main=reloaded_module.module) or ReloadModules(exclude=reloaded_module.module).

But I am of course open to renaming ReloadModules and reloaded_module too.

I have not perfected names and such in this PR, I wanted to initially share the concept.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

We can polish later :)

}

hotload(**conf)
Expand All @@ -237,3 +266,4 @@ def main():

if __name__ == "__main__":
main()