-
Notifications
You must be signed in to change notification settings - Fork 13.8k
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
chore: refactor file upload commands #28164
Conversation
"description": "If duplicate columns are not overridden," | ||
"they will be presented as 'X.1, X.2 ...X.x'." | ||
} | ||
) |
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.
@dpgaspar will this break an existing api integration if someone is passing this field? Should we deprecate it instead?
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 was only used on the new API introduced here: #27840
so this API was not yet released
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.
left one comment but it looks great!
SUMMARY
Small refactor on
CSVImportCommand
andExcelImportCommand
to DRY and make it more easily extendable and maintainable.Using dependency injection on a generic
UploadCommand
ofBaseReaders
that will be able to read a certain type of file.Pandas lib is able to read from multiple file types, CSV, HTML, Excel, XML, Latex, Feather, SAS XPORT etc. So with this refactor it will be much simpler and clean to add support for new file types.
Also this PR fixes the following bugs on reader options:
overrite_duplicates
: was an existing option that wasn't actually implemented, removed since I found no option for it on pandasread_csv
decimal
: was not actually implementedBEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION