[docs] initial docstring for makedataset #2879
Conversation
torchvision/datasets/folder.py
Outdated
| is_valid_file should not be passed. Defaults to None. | ||
|
|
||
| Raises: | ||
| ValueError: In case ``extensions`` and ``is_valid_file`` is None. |
There was a problem hiding this comment.
Technically, this is raised if both_none or both_something.
There was a problem hiding this comment.
addressed in a following commit
torchvision/datasets/folder.py
Outdated
| Args: | ||
| directory (str): root dataset directory | ||
| class_to_idx (Dict[str, int]): dictionary mapping class name to class index | ||
| extensions (Optional[Tuple[str, ...]], optional): A list of allowed extensions. |
There was a problem hiding this comment.
Optional is repeated twice now (here and below). I'm not sure if we started to add arg type as typing annotations... @pmeier what do you think ?
There was a problem hiding this comment.
I think we should stick to one form. Either
extensions (optional): A list of allowed extensions.or
extensions (Optional[Tuple[str, ...]]): Allowed extensions.I prefer the second version especially because the type hints can be auto generated from the annotations. That being said we need a sphinx extension for that and thus we should stick to the first version for now.
I've just realized that the type annotation and the docstring contradict each other. Should this be a list or a tuple?
There was a problem hiding this comment.
Fixed.
the latter type was auto-generated, sorry about that. Pushed the changes that hopefully resolve this :)
Please let me know if this is OK to merge :)
* initial docstring * Revert "initial docstring" This reverts commit 2bf68ca. * revert the formatting changes * clear up per Victor's comment * Addressing PR comments Co-authored-by: Bruno Korbar <korbar@vggdev9.vggdev.cluster>
* initial docstring * Revert "initial docstring" This reverts commit 2bf68ca. * revert the formatting changes * clear up per Victor's comment * Addressing PR comments Co-authored-by: Bruno Korbar <korbar@vggdev9.vggdev.cluster>
per issue
#2841
@vfdev-5 can you please check out that the wording is ok?