Skip to content
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

CLN: Start definition of Backend API, and implement version #2883

Merged
merged 10 commits into from
Aug 6, 2021
Merged

CLN: Start definition of Backend API, and implement version #2883

merged 10 commits into from
Aug 6, 2021

Conversation

datapythonista
Copy link
Contributor

@datapythonista datapythonista commented Jul 29, 2021

xref #2678

First step towards defining the Backend API properly with abstract methods (Backend will replace Client).

To make PRs small, I define a first set of methods that Backend/Client must have, and so far I implement version.

To be able to do this step by step, I move the implementation from Client to Backend, and I created the property in the base Client, calling the Backend implementation. Eventually, connect will return Backend instead of Client, and Client will be removed.

I'm making a backward-incompatible change here, returning version as a string, instead of a setuptools object. I think this will match user expactations, since libraries rarely parse it (i.e. any_library.__version__) and it will also make our life easier with type annotations (setuptools have different version objects that can be returned).

@datapythonista datapythonista added the refactor Issues or PRs related to refactoring the codebase label Jul 29, 2021
@datapythonista
Copy link
Contributor Author

@jreback if you don't mind prioritizing this one, this is blocking me from finishing the stuff for the release

Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

looks generally ok

docs/source/api.rst Show resolved Hide resolved
ibis/backends/base/__init__.py Show resolved Hide resolved
@datapythonista
Copy link
Contributor Author

@jreback whenever you've got time, would be great if you can check my comments. I think this should be ready, and it's blocking me from continuing with clarifying the API for the backends. Thanks!

@jreback jreback added this to the Next release milestone Aug 6, 2021
@jreback jreback merged commit a7c1c7e into ibis-project:master Aug 6, 2021
@jreback
Copy link
Contributor

jreback commented Aug 6, 2021

thanks @datapythonista prob should add a release note about the version string change next time.

@datapythonista
Copy link
Contributor Author

thanks @datapythonista prob should add a release note about the version string change next time.

Good point, added it in #2895, thanks for merging anyway.

@cpcloud cpcloud removed this from the Next release milestone Jan 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Issues or PRs related to refactoring the codebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants