add bytes and os.PathLike to ImageFolder type hint#4122
add bytes and os.PathLike to ImageFolder type hint#4122wztdream wants to merge 1 commit intopytorch:mainfrom
Conversation
|
Hi @wztdream! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks! |
| def __init__( | ||
| self, | ||
| root: str, | ||
| root: Union[str,bytes,os.PathLike], |
There was a problem hiding this comment.
As long as we only use it within os.path functions, it should be fine. Question is if we want to do that. I'm not sure if it is worth it.
There was a problem hiding this comment.
yeah, I feel like this isn't something we'd want to explicitly support.
Or at the very least, I'm worried that we'll have to update the type annotation every single time os.path decides to support yet another thing
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
pmeier
left a comment
There was a problem hiding this comment.
@wztdream Thanks for the PR! Unfortunately, you would need to fix the annotation in more places, since DataFolder internally uses functions that currently are only annotated to take str's. You can run mypy to detect all these cases.
|
@wztdream, If you are busy with some other works, then please let me know. I will be happy to make a PR for this issue :). |
Sorry for the delay, I am busy these days, it is great you can help to fix this issue, go ahead and thank you for help :) |
Sorry for delay, I just have some time to update this PR, but I found it seems modify the type hint not a good way to solve this issue, as vivekkumar7089 have mentioned, following is the errors after run mypy. The impact seems too large, I think we need better way to handle pathlike typehint. |
Hi all,
This is to add bytes and os.PathLike type to ImageFolder type hint to avoid type checking warning see issue