[doc][api] add a function to check if an api is really public #46261
Conversation
Signed-off-by: can <can@anyscale.com>
can-anyscale
changed the title
ci/ray_ci/doc/api.py
Outdated
| Check if this API has a private name. Private names are those that start with | ||
| underscores. | ||
| """ | ||
| return self.name.split(".")[-1].startswith("_") |
There was a problem hiding this comment.
there are many special functions in python that are actually public, but starts with _, like __del__, __iter__, __len__, __init__, __next__, etc, etc..
There was a problem hiding this comment.
and I am actually not sure about the annotation story for those in ray's api world..
There was a problem hiding this comment.
not sure about the annotation story for those in ray's api worl
that is fine, and why we need to agree on this api policy across teams (see line 8 which is our agreed definition of private, not public, etc.)
i will be implementing what is in this policy for now, and if the policy change i will update the code accordingly; at least this will allow me to turn on the lint check for data team who has aligned with this policy
There was a problem hiding this comment.
what does data team say about these api's? maybe ask them to have a look?
like at least __init__ method of a public api class needs to be public, right? do those also need to be explicitly annotated?
There was a problem hiding this comment.
currenly folks do not annotate the __init__ methods, or any other _ methods; instead the class needs to be annotated (e.g. https://github.com/ray-project/ray/blob/master/python/ray/data/dataset.py#L130)
existing logic for what is excluded from annotations https://github.com/ray-project/ray/blob/master/ci/lint/check_api_annotations.py#L42 (_ names and things under .internal.)
There was a problem hiding this comment.
in documentation, the class is used instead of the __init__ method as the constructor https://docs.ray.io/en/latest/data/api/dataset.html (e.g. ray.data.Dataset instead of ray.data.Dataset.__init__)
There was a problem hiding this comment.
at least this will allow me to turn on the lint check for data team who has aligned with this policy
The table overall looks good to me. To clarify, we're allowed to completely deprecate a stable API after 6 months warning, but we're not allowed to deprecate a parameter of a stable API with 6 months warning?
currenly folks do not annotate the init methods, or any other _ methods; instead the class needs to be annotated
Yeah, we don't typically annotate dunder method. It's conventional to document either __init__ or the class itself, but I think we annotate the class more often (if not always).
ci/ray_ci/doc/api.py
Outdated
| Check if this API has a private name. Private names are those that start with | ||
| underscores. | ||
| """ | ||
| return self.name.split(".")[-1].startswith("_") |
There was a problem hiding this comment.
what does data team say about these api's? maybe ask them to have a look?
like at least __init__ method of a public api class needs to be public, right? do those also need to be explicitly annotated?
Signed-off-by: can <can@anyscale.com>
Currently the `autosummary\n` will be strip the newline character, leads to incorrect parsing. The test case only works since it has double new lines. Fix the code + test case. Test: - CI Signed-off-by: can <can@anyscale.com>
| underscores. | ||
| """ | ||
| name_has_underscore = self.name.split(".")[-1].startswith("_") | ||
| is_internal = ".internal." in self.name |
There was a problem hiding this comment.
Think this has been addressed in a follow-up PR, but should be ._internal?
Add a function to check if an api name is prefixed with
_Also a function to check if an api is really public
Test: