Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
data pond: expose readable datasets as dataframes and arrow tables #1507
data pond: expose readable datasets as dataframes and arrow tables #1507
Changes from 30 commits
af6a40e
3a69ece
4324650
7c960df
86b89ac
5a8ea54
36e94af
20bf9ce
6dce626
a0ff55f
c297e96
d020403
5c3db47
79ef7dd
ff40079
ac415b9
c92a527
13ec73b
46e0226
7cf69a7
6ffe302
c200262
226454f
3296e63
6f6500f
152b788
6d73bc5
bdb39ba
3ead92b
9fcbd00
28ee1c6
28cb282
77926fa
6ef04bc
ec13b49
b222d1d
9f0a6a5
289b63c
779bca6
584ab47
6594053
9e0a61d
dd47326
9f8f79b
fda1cb5
c7a0e05
8497036
3dc2c90
d6bec38
62ea3ba
3fd4d61
eeca4ac
52f8523
90c669a
7657fb1
352b238
5fadeeb
9a1752d
a77192f
420eaf1
97e2757
df4f6d0
6b27b98
ffba901
fab5232
0de4a6c
077a25a
13a759b
10e04d6
5077ce1
1025560
f8927d3
7fd3c62
3a76178
27104e3
1c06d11
e5b3688
7d09bdb
dbe4baa
f4e0099
80fe898
d698cd5
857803c
8055529
24c7308
76759cf
d3d8381
f9a766d
b6c7fbc
86fc914
58380ec
0a24b3a
bef50d7
b13e492
92ea515
e1fa308
a7958d5
ed197ea
0ec1656
9cd4173
7dba771
3e96a6c
355f5b6
9002f02
c3050d4
bbc0525
ef148c3
a99e987
eaf1cd8
6bb7117
6648b86
89a9861
631d50b
8e2e37c
dc383fc
41926ae
9b8437a
5d14045
fb9a445
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
if we have
iter
we do not need to chunk right? maybelimit
would make sense? ormax_rows
? we are changing the semantics of the old method which could be also used to iterate but I'm OK with itThere 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 is a shorthand for getting a dataframe in one call, if you add a chunk_size only a chunk of that size will be collected.
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.
So I'm thinking about this interface all the time as well :) Maybe lets just expose
cursor()
here? which is a proper db api cursor (which we have already implemented)also: our implementation proxies all unknown methods to the underlying native cursor, did you notice?
so all methods below would be available via cursor
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.
I think I would keep it this way. The user may use the cursor too if they like, but this possiblity to iterate without having to open your own context manager is quite nice from the user perspective I think.
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.
I'd love to keep such stuff out of the common:
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.
I moved it around a bit. We have the dataaccess stuff in the destination, and imho it would be nice to have the correct typings there.
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.
my take: move it inti
dlt/destinations
that will remove a lot of the ugly inner importsThere 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 is moved
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.
OK already implemented