Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/replicate/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@
"Model",
"Version",
"ModelVersionIdentifier",
"get_path_url",
]

if not _t.TYPE_CHECKING:
Expand All @@ -104,6 +105,9 @@
for __name in __all__:
if not __name.startswith("__"):
try:
# Skip symbols that are imported later from _module_client
if __name in ("get_path_url", "run", "use"):
continue
__locals[__name].__module__ = "replicate"
except (TypeError, AttributeError):
# Some of our exported symbols are builtins which we can't set attributes for.
Expand Down Expand Up @@ -253,4 +257,5 @@ def _reset_client() -> None: # type: ignore[reportUnusedFunction]
collections as collections,
deployments as deployments,
predictions as predictions,
get_path_url as get_path_url,
)
1 change: 1 addition & 0 deletions src/replicate/_module_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

from . import _load_client
from ._utils import LazyProxy
from .lib._predictions_use import get_path_url # noqa: F401 # pyright: ignore[reportUnusedImport]
Copy link
Copy Markdown
Collaborator

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_url

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

avoid the lint comments

Ah nice. Fixed in 31536a8

What's the benefit of importing this function here just to re-export it somewhere else?

👇🏼 Here's Claude's rationalization for this change:

Looking at this, I can see that _module_client.py serves as a centralized export hub for all the main functions and resources that should be available at the top-level replicate module. Notice that run, use, files, models, etc. are all imported from _module_client.

The Architectural Benefits

  1. Consistent Pattern: All top-level exports follow the same path through _module_client.py
  2. Single Source of Truth: init.py only needs to import from one place
  3. Lazy Loading: The _module_client can implement lazy loading patterns for expensive resources
  4. API Boundary: It creates a clear separation between internal implementation (lib/) and public API

Could We Skip the Intermediary?

We could theoretically import directly in init.py:

from .lib._predictions_use import get_path_url as get_path_url

But this would break the established architectural pattern where all top-level functions go through module_client.py. Looking at the codebase, even simple functions like run and use follow this pattern.

The re-export isn't adding complexity - it's maintaining consistency with how this codebase organizes its public API exports. The pattern ensures that anyone working on the codebase knows exactly where to look for top-level exports: they all flow through _module_client.py.

@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!

Copy link
Copy Markdown
Collaborator

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__.py file is where all the main exports are, so we should just re-export directly from there instead of going through the module client file.



class FilesResourceProxy(LazyProxy["FilesResource"]):
Expand Down
4 changes: 4 additions & 0 deletions src/replicate/lib/_files.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,8 @@ class FileOutput(httpx.SyncByteStream):
def __init__(self, url: str, client: Replicate) -> None:
self.url = url
self._client = client
# Add __url__ attribute for compatibility with get_path_url()
self.__url__ = url

def read(self) -> bytes:
if self.url.startswith("data:"):
Expand Down Expand Up @@ -184,6 +186,8 @@ class AsyncFileOutput(httpx.AsyncByteStream):
def __init__(self, url: str, client: AsyncReplicate) -> None:
self.url = url
self._client = client
# Add __url__ attribute for compatibility with get_path_url()
self.__url__ = url

async def read(self) -> bytes:
if self.url.startswith("data:"):
Expand Down
106 changes: 106 additions & 0 deletions tests/lib/test_get_path_url.py
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