Update RemoteStore.__str__ and add UPath tests
#1964
Conversation
|
the test failures were due to a trailing "/" character added to the url / path attributes in the "init-from-UPath" code paths, and that's resolved via |
d-v-b
requested review from
jhamman and
martindurant
and removed request for
martindurant
| if isinstance(url, str): | ||
| self._fs, self.path = fsspec.url_to_fs(url, **storage_options) | ||
| self._url = url.rstrip("/") | ||
| self._fs, _path = fsspec.url_to_fs(url, **storage_options) |
There was a problem hiding this comment.
Is there a case where url has no trailing "/", but the return fom url_to_fs does?
There was a problem hiding this comment.
I am not aware of any, but I wanted to ensure that the UPath and non-UPath code paths do the exact same thing to the url string.
There was a problem hiding this comment.
I might be misunderstanding the question...
|
|
||
| def __str__(self) -> str: | ||
| return f"Remote fsspec store: {type(self._fs).__name__} , {self.path}" | ||
| return f"{self._url}" |
There was a problem hiding this comment.
We are implying that this store is the only way to access the URL displayed here
There was a problem hiding this comment.
yes, but this is consistent with our current practice for memory store and local store. Yes, this practice does mean that RemoteStore for local files will have the same str output as LocalStore, but I figure we should make the stores consistent before making them correct.
|
None of my comments are blockers |
* v3: (22 commits) [v3] `Buffer` ensure correct subclass based on the `BufferPrototype` argument (zarr-developers#1974) Fix doc build (zarr-developers#1987) Fix doc build warnings (zarr-developers#1985) Automatically generate API reference docs (zarr-developers#1918) Update `RemoteStore.__str__` and add UPath tests (zarr-developers#1964) [v3] Elevate codec pipeline (zarr-developers#1932) 0 dim arrays: indexing (zarr-developers#1980) `parse_shapelike` allows 0 (zarr-developers#1979) Clean up typing and docs for indexing (zarr-developers#1961) add json indentation to config (zarr-developers#1952) chore: update pre-commit hooks (zarr-developers#1973) Bump pypa/gh-action-pypi-publish in the actions group (zarr-developers#1969) chore: update pre-commit hooks (zarr-developers#1957) Update release.rst (zarr-developers#1960) doc: update release notes for 3.0.0.alpha (zarr-developers#1959) Basic working FsspecStore (zarr-developers#1785) Feature: Top level V3 API (zarr-developers#1884) Buffer Prototype Argument (zarr-developers#1910) Create issue-metrics.yml fixes bug in transpose (zarr-developers#1949) ...
2 participants
This PR changes the
.__str__method ofRemoteStoreto make it similar toLocalStoreandMemoryStore(i.e.,RemoteStore("s3://path").__str__returnss3://path).I also add tests for UPath, and tweaked the test suite accordingly. The tests are failing, and I don't know why. @martindurant could you take a look if you have time?edit: tests are passingTODO: