Andrew/372 check paths #382
Conversation
gvwilson
added
community
| @@ -10,6 +13,14 @@ | |||
| KJS_PATH = Path(__file__).resolve().parent / "vendor" / "kaleido_scopes.js" | |||
|
|
|||
|
|
|||
| def _ensure_path(path: Path | str): | |||
| _logger.debug(f"Ensuring path {path!s}") | |||
| if not urlparse(str(path)).scheme.startswith("file"): # is url | |||
There was a problem hiding this comment.
So, this logic as written doesn't actually resolve the issue in #372. AFAIK the plotly argument isn't required to start with "file://", so it can just be a "bare" path starting with e.g. "/Users/..." (on Mac). In that case, this if resolves to True and the function returns without error, even when the file doesn't actually exist.
This suggestion fixes that problem (assuming it's an OK assumption that the only things we consider URLs are paths which start with http or https).
| if not urlparse(str(path)).scheme.startswith("file"): # is url | |
| if urlparse(str(path)).scheme.startswith("http"): # is url |
| if not urlparse(str(path)).scheme.startswith("file"): # is url | ||
| return | ||
| if not Path(path).exists(): | ||
| raise ValueError(f"{path!s} does not seem to be a valid path.") |
There was a problem hiding this comment.
more assertive wording
and more specific exception type
| raise ValueError(f"{path!s} does not seem to be a valid path.") | |
| raise FileNotFoundError(f"{path!s} does not exist.") |
| force_cdn: (default False) Don't use plotly import, use CDN | ||
| plotly: The url to the plotly.js to use. Defaults to plotly.js | ||
| present in plotly.py, if installed. Otherwise fallback to | ||
| global constant. |
There was a problem hiding this comment.
could you edit this docstring to describe where to find these global constants? to help someone who might be trying to debug
There was a problem hiding this comment.
(not highest priority just trying to think about how to make these docstrings slightly less opaque)
| @@ -10,6 +13,14 @@ | |||
| KJS_PATH = Path(__file__).resolve().parent / "vendor" / "kaleido_scopes.js" | |||
|
|
|||
|
|
|||
| def _ensure_path(path: Path | str): | |||
| _logger.debug(f"Ensuring path {path!s}") | |||
| if not urlparse(str(path)).scheme.startswith("file"): # is url | |||
There was a problem hiding this comment.
Or perhaps this?
| if not urlparse(str(path)).scheme.startswith("file"): # is url | |
| path_scheme = urlparse(str(path)).scheme | |
| if path_scheme not in {"", "file"}: # is url |
solves #372