Feature/save #282
Feature/save #282
Conversation
| @@ -389,7 +392,7 @@ def summary(self): | |||
|
|
|||
| @property | |||
| def path(self): | |||
| return [self.username, self.project, self.name] | |||
| return [str(self.username), str(self.project), str(self.name)] | |||
There was a problem hiding this comment.
What would these be if not strings? None?
| for path in glob.glob(policy["glob"]): | ||
| save_name = os.path.relpath(path, self._watch_dir) | ||
| if self._file_event_handlers.get(save_name): | ||
| del self._file_event_handlers[save_name] |
There was a problem hiding this comment.
Is it safe to just delete these? What if it's one of the event handlers that has a thread?
Could you use a clearer name than update_policy? We're in the context of a RunManager and there are a million different types of policy someone could imagine a RunManager having. IMO it should at least say something about syncing files. I think short names are good for very commonly used public-facing functions, but for this internal stuff I think it's more important to make things readable for everyone else.
There was a problem hiding this comment.
Fixed in feature/html
| @@ -949,25 +971,42 @@ def _sync_etc(self, headless=False): | |||
|
|
|||
| exitcode = None | |||
| try: | |||
| payload = b'' | |||
There was a problem hiding this comment.
b'' doesn't work in Python 2. You can do six.b('').
There was a problem hiding this comment.
Python 2.7+ has support for the syntax for it.
| wandb.termerror(message) | ||
| util.sentry_message(message) | ||
| break | ||
| term = res.find(b'\0') |
There was a problem hiding this comment.
Should be six.b('\0') or something
| orig = data | ||
| if isinstance(data, list): | ||
| data = ints2bytes(data) | ||
| data = json.dumps(data).encode('utf8') + b'\0' |
| @@ -98,6 +102,21 @@ def _init_jupyter_agent(self): | |||
| def _stop_jupyter_agent(self): | |||
| self._jupyter_agent.stop() | |||
|
|
|||
| def configure(self, options): | |||
There was a problem hiding this comment.
Could you call this something more specific than "configure"? Especially since it seems like this actually only does only thing and the terminology here doesn't match what it's called in RunManager.
There was a problem hiding this comment.
Fixed in feature/html
| @@ -340,6 +359,10 @@ def history(self): | |||
| HISTORY_FNAME, self._dir, add_callback=self._history_added) | |||
| return self._history | |||
|
|
|||
| @property | |||
| def initial_step(self): | |||
There was a problem hiding this comment.
Is this used? Is it the initial step or the current step?
Saving now supports globs and specifying a policy. I added much better tests for run manager and a restore function as well to download a file in the event of resuming or pulling in weights from a different run.