feat: Rename FileExtensionRouter to FileTypeRouter, handle ByteStream(s)
#5998
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
|
cc @masci I named this component |
masci
suggested changes
Oct 9, 2023
ZanSara
reviewed
Oct 9, 2023
Comment on lines
52
to
79
| @pytest.mark.unit | ||
| def test_run_with_bytestreams(self, preview_samples_path): | ||
| """ | ||
| Test if the component runs correctly with ByteStream inputs. | ||
| """ | ||
| file_paths = [ | ||
| preview_samples_path / "txt" / "doc_1.txt", | ||
| preview_samples_path / "txt" / "doc_2.txt", | ||
| preview_samples_path / "audio" / "the context for this answer is here.wav", | ||
| preview_samples_path / "images" / "apple.jpg", | ||
| ] | ||
| mime_types = ["text/plain", "text/plain", "audio/x-wav", "image/jpeg"] | ||
| # Convert file paths to ByteStream objects and set metadata | ||
| byte_streams = [] | ||
| for path, mime_type in zip(file_paths, mime_types): | ||
| stream = ByteStream(path.read_bytes()) | ||
|
|
||
| stream.metadata["content_type"] = mime_type | ||
|
|
||
| byte_streams.append(stream) | ||
|
|
||
| router = FileTypeRouter(mime_types=["text/plain", "audio/x-wav", "image/jpeg"]) | ||
| output = router.run(sources=byte_streams) | ||
| assert output | ||
| assert len(output["text/plain"]) == 2 | ||
| assert len(output["audio/x-wav"]) == 1 | ||
| assert len(output["image/jpeg"]) == 1 | ||
| assert not output.get("unclassified", None) |
There was a problem hiding this comment.
Just for completeness, let's also add a ByteStream with no content_type and test that it goes under unclassified
| assert len(output["audio/x-wav"]) == 1 | ||
| assert len(output["image/jpeg"]) == 1 | ||
| assert not output.get("unclassified", None) | ||
|
|
There was a problem hiding this comment.
Let's add a test that mixes Paths and ByteStreams
dfokina
reviewed
Oct 9, 2023
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.
Why:
The main objective behind these commits was to refine the file routing mechanism by including support for
ByteStreamtypes. This initiative aims to broaden the spectrum of file types handled within the system, ensuring a more versatile and robust file routing solution.What:
FileExtensionRoutertoFileTypeRouterto better reflect its functionality.ByteStreamhandling within theFileTypeRouterrunmethod, thereby enhancing the routing capabilities to cater to byte stream data along with other file types.How Did You Test It:
Ran a suite of unit and integration tests to validate the updated routing mechanism. Additionally, performed manual testing to verify the correct routing of
ByteStreamdata and other file types through the updatedFileTypeRouter. All tests passed successfully, affirming the reliability and efficiency of the changes made.Notes for Reviewer:
ByteStreamhandling logic to ensure they align with the project's coding and naming conventions.ByteStreamis a crucial addition; any feedback on the implementation would be highly appreciated to ensure optimal performance and error handling.sourceinstead ofpathsin therunmethod signature. Aspathsis kinda specific, I thought thatsourcesbetter depicts this input parameter. We should standardize this name across all components that useUnion[str, Path, ByteStream]