-
Notifications
You must be signed in to change notification settings - Fork 2
feat: add get_path_url() function support for migration compatibility #61
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
Merged
Merged
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
ebde048
feat: add get_path_url() function support for migration compatibility
zeke 2a70529
refactor: simplify get_path_url import chain by removing intermediary
zeke 4b88305
fix: add bearer tokens to get_path_url tests to prevent auth errors
zeke 31536a8
refactor: use cleaner import pattern for get_path_url re-export
zeke 7a3f981
refactor: import get_path_url directly in __init__.py instead of via …
zeke 5b646a7
refactor: remove get_path_url from module_client skip list
zeke File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,106 @@ | ||
| from pathlib import Path | ||
|
|
||
| import replicate | ||
| from replicate.lib._files import FileOutput, AsyncFileOutput | ||
| from replicate.lib._predictions_use import URLPath, get_path_url | ||
|
|
||
| # Test token for client instantiation | ||
| TEST_TOKEN = "test-bearer-token" | ||
|
|
||
|
|
||
| def test_get_path_url_with_urlpath(): | ||
| """Test get_path_url returns the URL for URLPath instances.""" | ||
| url = "https://example.com/test.jpg" | ||
| path_proxy = URLPath(url) | ||
|
|
||
| result = get_path_url(path_proxy) | ||
| assert result == url | ||
|
|
||
|
|
||
| def test_get_path_url_with_fileoutput(): | ||
| """Test get_path_url returns the URL for FileOutput instances.""" | ||
| url = "https://example.com/test.jpg" | ||
| file_output = FileOutput(url, replicate.Replicate(bearer_token=TEST_TOKEN)) | ||
|
|
||
| result = get_path_url(file_output) | ||
| assert result == url | ||
|
|
||
|
|
||
| def test_get_path_url_with_async_fileoutput(): | ||
| """Test get_path_url returns the URL for AsyncFileOutput instances.""" | ||
| url = "https://example.com/test.jpg" | ||
| async_file_output = AsyncFileOutput(url, replicate.AsyncReplicate(bearer_token=TEST_TOKEN)) | ||
|
|
||
| result = get_path_url(async_file_output) | ||
| assert result == url | ||
|
|
||
|
|
||
| def test_get_path_url_with_regular_path(): | ||
| """Test get_path_url returns None for regular Path instances.""" | ||
| regular_path = Path("test.txt") | ||
|
|
||
| result = get_path_url(regular_path) | ||
| assert result is None | ||
|
|
||
|
|
||
| def test_get_path_url_with_object_without_target(): | ||
| """Test get_path_url returns None for objects without __url__.""" | ||
|
|
||
| # Test with a string | ||
| result = get_path_url("not a path") | ||
| assert result is None | ||
|
|
||
| # Test with a dict | ||
| result = get_path_url({"key": "value"}) | ||
| assert result is None | ||
|
|
||
| # Test with None | ||
| result = get_path_url(None) | ||
| assert result is None | ||
|
|
||
|
|
||
| def test_get_path_url_module_level_import(): | ||
| """Test that get_path_url can be imported at module level.""" | ||
| from replicate import get_path_url as module_get_path_url | ||
|
|
||
| url = "https://example.com/test.jpg" | ||
| file_output = FileOutput(url, replicate.Replicate(bearer_token=TEST_TOKEN)) | ||
|
|
||
| result = module_get_path_url(file_output) | ||
| assert result == url | ||
|
|
||
|
|
||
| def test_get_path_url_direct_module_access(): | ||
| """Test that get_path_url can be accessed directly from replicate module.""" | ||
| url = "https://example.com/test.jpg" | ||
| file_output = FileOutput(url, replicate.Replicate(bearer_token=TEST_TOKEN)) | ||
|
|
||
| result = replicate.get_path_url(file_output) | ||
| assert result == url | ||
|
|
||
|
|
||
| def test_fileoutput_has_url_attribute(): | ||
| """Test that FileOutput instances have __url__ attribute.""" | ||
| url = "https://example.com/test.jpg" | ||
| file_output = FileOutput(url, replicate.Replicate(bearer_token=TEST_TOKEN)) | ||
|
|
||
| assert hasattr(file_output, "__url__") | ||
| assert file_output.__url__ == url | ||
|
|
||
|
|
||
| def test_async_fileoutput_has_url_attribute(): | ||
| """Test that AsyncFileOutput instances have __url__ attribute.""" | ||
| url = "https://example.com/test.jpg" | ||
| async_file_output = AsyncFileOutput(url, replicate.AsyncReplicate(bearer_token=TEST_TOKEN)) | ||
|
|
||
| assert hasattr(async_file_output, "__url__") | ||
| assert async_file_output.__url__ == url | ||
|
|
||
|
|
||
| def test_urlpath_has_url_attribute(): | ||
| """Test that URLPath instances have __url__ attribute.""" | ||
| url = "https://example.com/test.jpg" | ||
| url_path = URLPath(url) | ||
|
|
||
| assert hasattr(url_path, "__url__") | ||
| assert url_path.__url__ == url |
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.
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.
What's the benefit of importing this function here just to re-export it somewhere else?
Also to avoid the lint comments you can export it with
import get_path_url as get_path_urlThere 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.
Ah nice. Fixed in 31536a8
👇🏼 Here's Claude's rationalization for this change:
@RobertCraigie does that rationalization make any sense to you? Or do you think we should just dip into the internal
lib._predictions_use? Please forgive my ignorance here around both this codebase and Python best practices!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.
That isn't quite accurate, the module client file is just for the
import replicate; replicate.models.list(), resource wrapping. The__init__.pyfile is where all the main exports are, so we should just re-export directly from there instead of going through the module client file.