[BEAM-6768] Sketching FileIO reading transforms for Python SDK #7791
Conversation
This file contains 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
|
Run Python PostCommit |
|
Ah yes, the file objects are not pickleable. Gotta work around that. |
|
@pabloem Could you explain the end goal here? Is this replacing PTransforms such as |
|
I'm working on a design doc for it, but I was trying to get away with a PR
before the design doc:)- I'll share the design doc soon
…On Thu, Feb 14, 2019, 6:21 PM Udi Meiri ***@***.*** wrote:
@pabloem <https://github.com/pabloem> Could you explain the end goal
here? Is this replacing PTransforms such as ReadFromText?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#7791 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ABPc7P8OWcU05OI7wX1OJxUDqdP0hRZ2ks5vNhm4gaJpZM4axTBW>
.
|
|
Soon meaning tomorrow*
…On Thu, Feb 14, 2019, 7:13 PM Pablo Estrada ***@***.*** wrote:
I'm working on a design doc for it, but I was trying to get away with a PR
before the design doc:)- I'll share the design doc soon
On Thu, Feb 14, 2019, 6:21 PM Udi Meiri ***@***.*** wrote:
> @pabloem <https://github.com/pabloem> Could you explain the end goal
> here? Is this replacing PTransforms such as ReadFromText?
>
> —
> You are receiving this because you were mentioned.
>
>
> Reply to this email directly, view it on GitHub
> <#7791 (comment)>, or mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/ABPc7P8OWcU05OI7wX1OJxUDqdP0hRZ2ks5vNhm4gaJpZM4axTBW>
> .
>
|
|
@udim A couple examples are for consuming CSV files, where we need to use the first line of every file as header (this is not well supported by textio), and also returning PCollections with (file_name, line) pairs, which has also been asked about before |
|
Run Python PostCommit |
|
PTAL? : ) |
udim
requested changes
Mar 1, 2019
| elif setting == EmptyMatchTreatment.ALLOW_IF_WILDCARD and '*' in pattern: | ||
| return True | ||
| else: | ||
| return False |
There was a problem hiding this comment.
Do we also want to validate that setting has a valid value? (e.g.:
elif setting == EmptyMatchTreatment.DISALLOW:
return False
else:
raise ValueError(setting)). I'm thinking about future changes adding/removing enum values.
There was a problem hiding this comment.
That makes sense. Thanks Udi.
| and its metadata; and ``ReadAll``, which takes in a ``PCollection`` of file | ||
| metadata records, and produces a ``PCollection`` of (file metadata, file handle) | ||
| tuples. | ||
| These transforms currently do not support splitting by themselves. |
There was a problem hiding this comment.
Do we want mark this whole module as experimental for now?
sdks/python/apache_beam/io/fileio.py
Outdated
| This ``PTransform`` returns a ``PCollection`` of matching files in the form | ||
| of ``FileMetadata`` objects.""" | ||
|
|
||
| def __init__(self, file_pattern, empty_match_treatment=None): |
There was a problem hiding this comment.
Document the default value and simplify the code setting self._empty_match_treatment:
Suggested change
| def __init__(self, file_pattern, empty_match_treatment=None): | |
| def __init__(self, file_pattern, empty_match_treatment=EmptyMatchTreatment.ALLOW_IF_WILDCARD): |
sdks/python/apache_beam/io/fileio.py
Outdated
| This ``PTransform`` returns a ``PCollection`` of matching files in the form | ||
| of ``FileMetadata`` objects.""" | ||
|
|
||
| def __init__(self, empty_match_treatment=None): |
There was a problem hiding this comment.
Suggested change
| def __init__(self, empty_match_treatment=None): | |
| def __init__(self, empty_match_treatment=EmptyMatchTreatment.ALLOW): |
sdks/python/apache_beam/io/fileio.py
Outdated
| Provides thre reading ``PTransform``\\s, ``MatchFiles``, | ||
| ``MatchAll``, that produces a ``PCollection`` of records representing a file | ||
| and its metadata; and ``ReadAll``, which takes in a ``PCollection`` of file | ||
| metadata records, and produces a ``PCollection`` of (file metadata, file handle) |
There was a problem hiding this comment.
produces a PCollection of ReadableFiles?
sdks/python/apache_beam/io/fileio.py
Outdated
|
|
||
| Provides thre reading ``PTransform``\\s, ``MatchFiles``, | ||
| ``MatchAll``, that produces a ``PCollection`` of records representing a file | ||
| and its metadata; and ``ReadAll``, which takes in a ``PCollection`` of file |
pabloem
commented
Mar 5, 2019
sdks/python/apache_beam/io/fileio.py
Outdated
|
|
||
| Provides thre reading ``PTransform``\\s, ``MatchFiles``, | ||
| ``MatchAll``, that produces a ``PCollection`` of records representing a file | ||
| and its metadata; and ``ReadAll``, which takes in a ``PCollection`` of file |
sdks/python/apache_beam/io/fileio.py
Outdated
| Provides thre reading ``PTransform``\\s, ``MatchFiles``, | ||
| ``MatchAll``, that produces a ``PCollection`` of records representing a file | ||
| and its metadata; and ``ReadAll``, which takes in a ``PCollection`` of file | ||
| metadata records, and produces a ``PCollection`` of (file metadata, file handle) |
| elif setting == EmptyMatchTreatment.ALLOW_IF_WILDCARD and '*' in pattern: | ||
| return True | ||
| else: | ||
| return False |
There was a problem hiding this comment.
That makes sense. Thanks Udi.
sdks/python/apache_beam/io/fileio.py
Outdated
| This ``PTransform`` returns a ``PCollection`` of matching files in the form | ||
| of ``FileMetadata`` objects.""" | ||
|
|
||
| def __init__(self, file_pattern, empty_match_treatment=None): |
sdks/python/apache_beam/io/fileio.py
Outdated
| This ``PTransform`` returns a ``PCollection`` of matching files in the form | ||
| of ``FileMetadata`` objects.""" | ||
|
|
||
| def __init__(self, empty_match_treatment=None): |
| and its metadata; and ``ReadAll``, which takes in a ``PCollection`` of file | ||
| metadata records, and produces a ``PCollection`` of (file metadata, file handle) | ||
| tuples. | ||
| These transforms currently do not support splitting by themselves. |
pabloem
changed the title
|
Run Python PostCommit |
|
@udim PTAL : ) |
|
LGTM, thanks |
|
Run Portable_Python PreCommit |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
r: @chamikaramj
These transforms sketch the reading transforms from
FileIO. I'd love for you tot ake a look Cham : ) - They need better documentation, which I'm planning to add, but I wanted to see what you think of what I have currently.Design doc: https://s.apache.org/fileio-beam-python