-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Show Create DB log in dataset view #222
Conversation
@@ -53,9 +53,13 @@ def __init__(self, input_file, db_name, image_dims, **kwargs): | |||
|
|||
self.entries_count = None | |||
self.distribution = None | |||
self.create_db_log_file = "createdb_%s_output.txt" % db_name |
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.
That's a little wordy. Would create_%s.log
be too short?
Great PR! This should definitely help with some of the confusion about DIGITS silently ignoring errors when creating datasets. |
assert path and path != '' | ||
assert os.path.isfile(path), "log file %s does not exist" % path | ||
assert self.delete_dataset(job_id) == 200, 'delete failed' | ||
assert not self.dataset_exists(job_id), 'dataset exists after delete' |
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.
No need to explicitly delete the dataset. It will be deleted in tearDownClass()
:
DIGITS/digits/dataset/images/classification/test_views.py
Lines 82 to 84 in f3c4078
# delete any created datasets | |
for job_id in cls.created_datasets: | |
cls.delete_dataset(job_id) |
ce544fe
to
a915e24
Compare
Thanks for the feedback Luke! I have made the changes you suggested. I am not sure why code coverage went down. The tool reports a miss on lines https://github.com/gheinrich/DIGITS/blob/dev/createDbLog/digits/dataset/images/classification/test_views.py#L267-L274 even though they are part of a test that passed.
I leave this decision to you :-) |
I've noticed that there are some lines in the scheduler which run sometimes and not others. It's not really a problem, but it makes our test coverage vary somewhat from run to run.
Yeah that's weird. |
@@ -53,9 +53,13 @@ def __init__(self, input_file, db_name, image_dims, **kwargs): | |||
|
|||
self.entries_count = None | |||
self.distribution = None | |||
self.create_db_log_file = "create_%s.txt" % db_name |
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.
Can we use .log
instead of .txt
? That's what we have for the Caffe output.
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.
Yes, sorry I read the first comment too quickly. I used the .txt extension because in Firefox it shows the file automatically without asking the user to download/choose an external tool to open it. But you're right, it's better to keep consistency in the project.
a915e24
to
ffc8f05
Compare
Show Create DB log in dataset view
No description provided.