Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

fromfile warning for using valid/test #3063

Merged
merged 3 commits into from
Sep 9, 2020
Merged

fromfile warning for using valid/test #3063

merged 3 commits into from
Sep 9, 2020

Conversation

jaseweston
Copy link
Contributor

No description provided.

@github-actions
Copy link

github-actions bot commented Sep 9, 2020

Your PR contains a change to a task. Please paste the results of the following command into a comment:

python tests/datatests/test_new_tasks.py

@@ -83,6 +84,9 @@ def __init__(self, opt, shared=None):
datafile = opt['fromfile_datapath']
if self.opt['fromfile_datatype_extension']:
datafile += "_" + self.opt['datatype'].split(':')[0] + '.txt'
else:
if self.opt['datatype'] == 'valid' or self.opt['datatype'] == 'test':
logging.warn('You are using this fromfile data as a valid or test set without setting fromfile_datatype_extension to true. Please be aware this uses directly the file you indicated, make sure this is not the same as your training file.')
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's also going to print Batchsize number times. can you add a "if shared is None" protection.

Also I don't think it will handle valid:stream right but that's unusual in fromfile anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

Copy link
Contributor

@stephenroller stephenroller left a comment

Choose a reason for hiding this comment

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

Cheers

@jaseweston jaseweston merged commit 5265587 into master Sep 9, 2020
@jaseweston jaseweston deleted the warning branch September 9, 2020 21:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants