Skip to content

Autodetect the engine only when engine=None #4458

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

Merged
merged 18 commits into from
Sep 28, 2020
Merged

Autodetect the engine only when engine=None #4458

merged 18 commits into from
Sep 28, 2020

Conversation

alexamici
Copy link
Collaborator

@alexamici alexamici commented Sep 25, 2020

Always use the engine explicitly selected by the user and perform auto detection only when engine=None.

The refactor complexity stems mostly from moving or duplicating some logic from _get_engine_from_magic_number to the backend stores. The current, rather ugly, code minimizes changes to the error conditions and messages.

Later in the backend refactor the plugins will provide the functionalities to identify the format from the file content and the duplication should be eliminated.

@jhamman do you think I need to acknowledge the CZI sponsorship in the changelog?

@alexamici alexamici marked this pull request as ready for review September 25, 2020 10:18
@alexamici alexamici changed the title WIP: Autodetect the engine only when engine=None Autodetect the engine only when engine=None Sep 25, 2020
Copy link
Member

@jhamman jhamman left a comment

Choose a reason for hiding this comment

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

Looks great. Just a few small comments.

@alexamici
Copy link
Collaborator Author

alexamici commented Sep 28, 2020

@jhamman & @shoyer ping.

We'd like to have this (and optionally #4462) merged soon so we can more easily work on PR from master. Thanks!

@jhamman
Copy link
Member

jhamman commented Sep 28, 2020

I'm going to merge this now. The doc build failure is unrelated to the changes here.

@jhamman jhamman merged commit f821fe2 into pydata:master Sep 28, 2020
@alexamici alexamici deleted the autodetect_engine_refactor branch September 28, 2020 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Silently ignoring user passed engine key in some cases when opening file objects
3 participants