Fix bug if string is source in fetch_dataset_filepaths #247
Conversation
albrja
requested review from
hussain-jafari,
mattkappel,
ramittal,
rmudambi and
stevebachmeier
as code owners
src/pseudopeople/interface.py
Outdated
| @@ -469,7 +469,9 @@ def validate_data_path_suffix(data_paths) -> None: | |||
| return None | |||
|
|
|||
|
|
|||
| def get_dataset_filepaths(source: Path, dataset_name: str) -> List[Path]: | |||
| def get_dataset_filepaths(source: Union[Path, str], dataset_name: str) -> List[Path]: | |||
There was a problem hiding this comment.
You don't actually need to do if type(source) == str. You can just Path(source) and even if it's already a Path it'll work.
Also, I think this is the wrong place to do this. If you follow the trail up (and the type hints), I think you should convert to Path at the start of _generate_dataset at the top of this script. In that way from there on out it's always a Path object and the type hints are all correct (including in the fetch_filepaths function which is where this get_dataset_filepaths function gets called)
There was a problem hiding this comment.
I was debating which one of these was the better way to do it.
| @@ -46,6 +46,8 @@ def _generate_dataset( | |||
|
|
|||
| if source is None: | |||
| source = paths.SAMPLE_DATA_ROOT | |||
| else: | |||
There was a problem hiding this comment.
You don't even technically need the else: but this is fine
Bugfix/mic-4364
Fixes bug in fetch_dataset_filepaths
-handles conversion of strings to path objects
Testing
I am able to run generate_XXX providing a string for the source argument.