-
Notifications
You must be signed in to change notification settings - Fork 133
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
Fix path metadata #686
Fix path metadata #686
Conversation
8345e45
to
fc31a71
Compare
hamilton/io/utils.py
Outdated
if isinstance(path, Path): | ||
path = str(path) | ||
parsed = parse.urlparse(path) | ||
size = 0 |
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.
this should be None or -1. 0 could be a real value.
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.
Fair, I think None
is probably fine
fc31a71
to
4e6d4dd
Compare
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 add test that checks scheme & notes
Pandas and other tools can read from, say, s3. Our get_file_metadata will currently break. This fixes it, adding default values + a note that says that metadata is not supported. We will want to add more metadata gatherer plugins later, but this is not necessary.
4e6d4dd
to
5e8122e
Compare
Fixes path metadata for s3/other URLs.
Changes
How I tested this
Notes
Checklist