Skip to content

Conversation

@tuliocasagrande
Copy link
Contributor

Description of changes:
We are currently manipulating each pandas.read_csv parameter individually and setting default values in case they were missing. A safer approach would be to forward them and let pandas handle default values.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

ini -= forgotten_bytes
end -= 1 # Range is inclusive, contrary from Python's List
bytes_range = "bytes={}-{}".format(ini, end)
bytes_range = f"bytes={ini}-{end}"
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

:param sql: SQL Query
:param database: Glue/Athena Database
:param s3_output: AWS S3 path
:param s3_output: Amazon S3 path
Copy link
Contributor

Choose a reason for hiding this comment

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

Oops

thousands, decimal, lineterminator, quotechar, quoting, escapechar, parse_dates,
infer_datetime_format, na_values, keep_default_na, na_filter, encoding, converters),
)
proc = mp.Process(target=self._read_csv_once_remote,
Copy link
Contributor

Choose a reason for hiding this comment

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

Much better now!

Copy link
Contributor

@igorborgest igorborgest left a comment

Choose a reason for hiding this comment

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

Great contribution, thanks @tuliocasagrande!

@igorborgest igorborgest added the enhancement New feature or request label Jan 31, 2020
@tuliocasagrande tuliocasagrande merged commit 74896f0 into master Jan 31, 2020
@tuliocasagrande tuliocasagrande deleted the pandas_kwargs branch January 31, 2020 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants