Change out_dir to better reflect the fs needs
#486
Conversation
emcastillo
changed the title
out_dir to better reflect the fs needsout_dir to better reflect the fs needs
emcastillo
changed the title
out_dir to better reflect the fs needsout_dir to better reflect the fs needs
|
I think it looks a bit better now, but still not 100% happy @belltailjp , do you have any suggestions 😇 ? |
|
Thanks so much for the quick prototype!!
Right... I feel that the responsibility to handle where to save things into is currently a bit diverged ( writer = SimpleWriter(PosixFileSystem("/path/to/some/dir")) |
| @@ -89,6 +93,7 @@ def list( | |||
| if path_or_prefix is None: | |||
| raise ValueError( | |||
| "'path_or_prefix' must not be None in recursive mode.") | |||
| path_or_prefix = self.get_actual_path(path_or_prefix) | |||
There was a problem hiding this comment.
Need to normalize path even if the recursive=False case.
| return os.makedirs(file_path, mode, exist_ok) | ||
|
|
||
| def exists(self, file_path: str) -> bool: | ||
| file_path = self.get_actual_path(file_path) | ||
| return os.path.exists(file_path) | ||
|
|
||
| def rename(self, src: str, dst: str) -> None: |
There was a problem hiding this comment.
good catch, old code
|
PTAL 😄 |
| @@ -89,11 +89,11 @@ def list( | |||
| path_or_prefix: Optional[str] = None, | |||
| recursive: bool = False, | |||
| ) -> Iterator[str]: | |||
| path_or_prefix = self.get_actual_path(path_or_prefix) | |||
There was a problem hiding this comment.
get_actual_path could be None here 😅
There was a problem hiding this comment.
yeah, I think this should fix it, get_actual_path fails and os.scandir admits None
|
/test mini |
Closes #475
out_dirhas now a default value of '.' since using an empty one required major changes.I am not happy at all of how this PR turn out to be, I think we may need to rethink how fs and out_dir interact