| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
FYI @renato2099 getting the python based providers ended up being a blocking issue for some of my work so I took a stab at implementing it. Please tell me what you think if you have some time. |
Sorry, something went wrong.
|
Hi @timsaucer , I am sorry I wasn't able to complete this in time, but I had it still on my radar. I pushed my version yesterday after the holidays :) #1137 and after a quick look at your PR, it seems they are relatively similar. |
Sorry, something went wrong.
|
Hi @timsaucer , thanks for pushing on this, I think the main difference is that you are extending the PyCatalog whereas I am introducing a new PyCatalogProvider. Both approaches have their pros/cons, imho keeping the separation between catalogs and catalog_providers might make things clearer at the interface level. That said I don't have a feel what datafusion-python users might find better, i.e., more rigid+"clearer" APIs vs more fluid+a bit less clearer API by accepting different object types (catalogs and catalog_providers when registering providers). Maybe it is more pythonic to accept different object types? PR looks good to me overall though, thanks again! and sorry for the delay on my side :( |
Sorry, something went wrong.
There was a problem hiding this comment.
Thank you @timsaucer and @renato2099 🚀
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Which issue does this PR close?
Closes #1091
Rationale for this change
This PR builds on top of #1137 and adds python based schema and catalog providers.
What changes are included in this PR?
Creates wrapper types around python based schema and catalog providers.
Adds checks for when we are going python->rust->python and short circuit to return the python original object rather than wrapper.
Adds unit test
Are there any user-facing changes?
No
Status
Leaving as draft until the following are complete: