Add torchvision maintainers guide #7109
Conversation
maintainer_guide.md
Outdated
|
|
||
| ### What is public and what is private? | ||
|
|
||
| For the Python API, torchvision largely follows the [torch core |
There was a problem hiding this comment.
I'm never sure how we should call core: torch or PyTorch?
There was a problem hiding this comment.
over here it's usually "torch core" or "core", but "PyTorch" isn't ambiguous either. Happy with whatever you prefer
There was a problem hiding this comment.
In that case, I'm in favor of "torch" since with that we don't have a disconnect in the naming scheme when talking about "torchvision". (I really hate that in conda install pytorch torchvision -c pytorch)
There was a problem hiding this comment.
Sorry to be pedantic but "torch" may refer to the original Lua torch ? https://en.wikipedia.org/wiki/Torch_(machine_learning)
maintainer_guide.md
Outdated
| - objects that start with a leading underscore are private unless they're | ||
| exposed / aliased as public in a public `__init__.py` file |
There was a problem hiding this comment.
Are you referring to the situation discussed offline, i.e. defining public function in a private namespace and expose them in a public one? If so, can we clarify the text? The "exposed / aliased" implies for me that if we expose _foo in a public __init__.py, it can be considered public. Was that your intention?
There was a problem hiding this comment.
Happy to clarify, I tried "exposed / aliased as public" but maybe it's not clear enough. Perhaps
exposed / aliased as public (i.e. without a leading underscore) in a public
__init__.pyfile
?
There was a problem hiding this comment.
Maybe a simpler way is to just say:
Any object exposed in a public
__init__.pyis public unless it has a leading underscore in its immediate name
?
There was a problem hiding this comment.
Yes, I like that a lot better
There was a problem hiding this comment.
So it's just a special case of what we have above:
- objects in a public file that don't have a leading underscore are public
Do you think it's worth re-iterating that with __init__.py ?
There was a problem hiding this comment.
Technically yes, since __init__.py itself is not a public file given its underscores. But since have stated above that our definition might technically not be perfect and this should be common knowledge for Python devs, I'm ok to merge the two.
Co-authored-by: Philip Meier <github.pmeier@posteo.de>
Co-authored-by: Philip Meier <github.pmeier@posteo.de>
maintainer_guide.md
Outdated
| ### Deprecation policy. | ||
|
|
||
| Because they're disruptive, **deprecations should only be used sparingly**. | ||
|
|
||
| We largely follow the [torch core | ||
| policy](https://github.com/pytorch/pytorch/wiki/PyTorch's-Python-Frontend-Backward-and-Forward-Compatibility-Policy): | ||
| breaking changes require a deprecation period of at least 2 versions. |
There was a problem hiding this comment.
I would assume that this document is not intended to users. However this part and other part about C++ being private, it worth exposing to the users.
There was a problem hiding this comment.
I agree...
At the same time, I don't know where we could expose it properly. Pytorch typically doesn't have that in the online docs (I think?), it's rather in the Wiki which most users wouldn't unless they're actively searching for it.
Perhaps changing the title from maintiner_guie to policies or something like that? Happy to hear any other suggestion
Co-authored-by: Philip Meier <github.pmeier@posteo.de>
|
Hey @NicolasHug! You merged this PR, but no labels were added. The list of valid labels is available at https://github.com/pytorch/vision/blob/main/.github/process_commit.py |
NicolasHug
added
the
other
Summary: Co-authored-by: Philip Meier <github.pmeier@posteo.de> Reviewed By: vmoens Differential Revision: D43116115 fbshipit-source-id: c5c343ae1aecfbc4779fc88c16369f53f397d30c
This guide properly documents our deprecation policy (which was introduced a while ago) and establishes the distinction between public and private APIs - following the torch core policy.
Rendered version
CC @pmeier