Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Python Dataset Reader #2414
Python Dataset Reader #2414
Changes from all commits
746650a
64a76e6
7eb92a6
9a7a150
8fc35e7
e6465c5
c20d4b4
88c52bd
6027f19
05d1395
faee18a
4702069
9e4cba8
44eda6d
a2f9a15
ac2d403
5cf06ab
753a52f
944b90f
de0e5fa
0af89f8
fa668c9
6c33696
63288ac
75aac4e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Why not use the pytest version?
Would be nice to extend the test infrastructure (maybe at a later PR?)
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 you clarify what you mean here?
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.
Certainly. What I meant is that our pytest version of the tests (i.e.,
@test_util.lbann_test
) might have a nicer syntax here. However, it is designed to use the existing Python data reader and may be extended as such to use the dataset reader (for example, with a flag@test_util.lbann_test(dataset=MyObject())
or similar).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.
I agree we should change the test framework to use the new python dataset reader, but I think that should be a separate PR.