-
Notifications
You must be signed in to change notification settings - Fork 6
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
Remove unnecessary filename field in FileData #173
Conversation
LGTM. Would wait for @nv-hwoo to take a look and approve/comment accordingly. |
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.
Just one nit suggestion to update the docstring. Otherwise, looks great 👍
Thanks, done! |
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.
Nice, LGTM!
Remove the field in FileData that holds a reference to the file's name. It is not needed, since GenericDatastructure holds a dictionary of filenames to file data and is the way that file data are referenced. The field results in duplication, which could mask bugs.