feat(python): support shallow_clone#4949
Conversation
8326f30 to
81cdcdc
Compare
f18f27e to
1eeb208
Compare
1eeb208 to
e66f684
Compare
…atisfy &mut self; test: skip memory:// case under current env and assert using cloned objects
…allow_clone and apply ruff formatting
…; revert Cargo.lock to origin
… in one test; ensure Windows-compatible
62f77eb to
6acad63
Compare
|
Ready for review @jackye1995 |
924deb2 to
fa61802
Compare
| ---------- | ||
| target_path : str or Path | ||
| The URI or filesystem path to clone the dataset into. | ||
| version : int or str |
There was a problem hiding this comment.
this is not up to date with the Union[int, str, Tuple[int, str]]
|
|
||
| new_inner = self._ds.shallow_clone(target_uri, version, storage_options) | ||
|
|
||
| ds = LanceDataset.__new__(LanceDataset) |
There was a problem hiding this comment.
can we just directly load a new dataset, in stead of overriding these parameters? It feels fragile if we add anything new to LanceDataset that needs override.
There was a problem hiding this comment.
Sounds reasonable.
So we just ignore the shallow_clone returned dataset right? the manifest is cached anyway. Just make sure this is noticed
There was a problem hiding this comment.
We could have some static method like LanceDataset.clone_from to do it, but I feel it is not worth the complexity, but let me know if you disagree.
But this reminds me that, we should add a kwarg so it can pass in whatever other parameters (e.g. read_params, default_scan_options) that the user wants to open the cloned dataset.
There was a problem hiding this comment.
Done, PTAL when you have time
|
Error unrelated to this PR and exists on main branch, merging |
Close lance-format#4856 --------- Co-authored-by: majin.nathan <majin.nathan@bytedance.com>
Close #4856