Expose datasource to python - #632
Merged
Merged
Conversation
As we extend our support of i/o media, some information needs to be generalized as currently it is specific to files only.
ajaust
reviewed
Jul 7, 2025
ajaust
left a comment
ajaust
left a comment
Contributor
There was a problem hiding this comment.
The changes look good to me overall. 🙂
I added quite some questions for clarification again, but I think its all things and nothing serious.
Comment on lines
+186
to
+188
| Configuration for certain algorithms. True to minimize the number of | ||
| requests to the stream at the cost of reading/writing unnecessary data. | ||
| False to minimize memory usage at the cost of many requests. |
Contributor
There was a problem hiding this comment.
If we update the docstring in create_with this should be updated accordingly.
achaikou
commented
Jul 8, 2025
achaikou
left a comment
achaikou
left a comment
Contributor
Author
There was a problem hiding this comment.
Thanks for the fast review! 🐱
Some few things are updated, some are waiting your opinion :)
ajaust
reviewed
Jul 8, 2025
ajaust
left a comment
ajaust
left a comment
Contributor
There was a problem hiding this comment.
Great! I have only minor comments/answers to the open discussions and found a typo.
achaikou
commented
Jul 8, 2025
achaikou
left a comment
achaikou
left a comment
Contributor
Author
There was a problem hiding this comment.
Thanks! Finding typos is good, finding typos I like!
ajaust
approved these changes
Jul 9, 2025
ajaust
left a comment
ajaust
left a comment
Contributor
There was a problem hiding this comment.
Awesome. Go ahead and squash the fixups and merge. 😄
- Stream datasources are tested in stream.py file. File is supposed to be referenced in tutorials for users who want to implement their own streams. Tests are based on new tiny.sgy file. File is added to support cloud testing and cloud operations can be very very slow. Even for this tiny file "read" test sends almost 200 requests to the source. - Memory buffer datasource is mostly tested in its own file which tests are subsets of stream tests. Benchmark tests are already updated to support datasources, but workflow will be added later.
achaikou
force-pushed
the
python_datasource
branch
from
July 9, 2025 10:56
737154a to
3b960a7
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Datasource finally reaching the users. 🏁
Please double check that I am providing them with enough correct and useful information 😇 and that this solution is good enough and works as I think it does 😅 .
Only thing left is benchmark workflow, but I want to add it after this PR is merged.
Relevant for #573