-
Notifications
You must be signed in to change notification settings - Fork 50
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
Added handling of conversion of names #34
Conversation
# will not be accepted by `bq load` when it contains illegal characters. | ||
# Characters such as #, / or -. Neither will it be accepted if the column name | ||
# in the schema is larger than 128 characters. | ||
self.sanitize_names = sanitize_names |
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 remove the trailing white space 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.
Excellent PR, I appreciate that you added unit tests. Just a bunch of small nits below.
# Characters such as #, / or -. Neither will it be accepted if the column name | ||
# in the schema is larger than 128 characters. | ||
self.sanitize_names = sanitize_names | ||
|
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.
Here too, remove trailing white spaces..
elif key == 'type' and value in ['QINTEGER', 'QFLOAT', 'QBOOLEAN']: | ||
new_value = value[1:] | ||
elif key == 'mode': | ||
if infer_mode and value == 'NULLABLE' and filled: | ||
new_value = 'REQUIRED' | ||
else: | ||
new_value = value | ||
elif key == 'name' and sanitize_names: | ||
print(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.
Remove debugging print() statement, will corrupt the schema output
tests/test_generate_schema.py
Outdated
records = chunk['records'] | ||
expected_errors = chunk['errors'] | ||
expected_error_map = chunk['error_map'] | ||
expected_schema = chunk['schema'] | ||
|
||
print("Test chunk %s: First record: %s" % (chunk_count, records[0])) | ||
|
||
print(sanitize_names) |
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.
Remove debugging print() statement
Hi again. It should look better now, let me know if there is anything else you think I should change. |
There is still one line that has a trailing space, but it's minor. I'll fix it after merging this, and I'll update the CHANGELOG and README. Thanks for the PR! |
Thank you, I really like the work you put in. |
when given a csv with incorrect column names
closes issue #14
#33