-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Conversation
allennlp/commands/train.py
Outdated
if os.path.exists(serialization_dir) and os.listdir(serialization_dir): | ||
if not recover: | ||
if force: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The level of nesting that this gets to is scary. I also think you can avoid it - what if you just move the remove block up above, and leave the rest of the code unmodified? That is, have something like:
if recover and force:
raise ...
if force:
shutil.rmtree(serialization_dir)
... # the rest of the code unchanged
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah--I'm fine with that.
If I omit the exists check then I need to tell |
Yeah, keeping that exists check is fine, I was more commenting on pulling out that branch to up above, not on the specifics of the code block. Looks like you forgot to move back the main logic that you changed, though. |
👍 |
Fixes #1575