-
Notifications
You must be signed in to change notification settings - Fork 13.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FLINK-37365][pyflink] Add API stability decorators for PyFlink APIs #26247
base: master
Are you sure you want to change the base?
Conversation
I only added the annotations to the Table side because im a little more familiar with them than the datastream side and thought it would be good to gather feedback. I can add it as part of this PR or as a followup. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome work @autophagy. The docs looks very nice. I found some missing py files that we should also annotate:
- deprecate
descriptor
- internalize
functions
,utils
, maybe more? - annotate
expression
,expressions
@@ -34,6 +37,7 @@ def __init__(self, j_module): | |||
self._j_module = j_module | |||
|
|||
|
|||
@PublicEvolving() | |||
class HiveModule(Module): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure why this is here. The Hive module got factored out of the Flink core repository? Maybe a followup issue to remove it? Maybe don't annotate it?
@@ -959,14 +962,12 @@ def to_pandas(self): | |||
import pandas as pd | |||
return pd.DataFrame.from_records([], columns=self.get_schema().get_field_names()) | |||
|
|||
@Deprecated(since="2.1.0", detail=":func:`Table.get_resolved_schema` instead.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Deprecated(since="2.1.0", detail=":func:`Table.get_resolved_schema` instead.") | |
@Deprecated(since="2.1.0", detail="Use :func:`Table.get_resolved_schema` instead.") |
@@ -36,6 +37,7 @@ | |||
'Constraint', 'UniqueConstraint', 'ResolvedSchema'] | |||
|
|||
|
|||
@PublicEvolving() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great that there is @PublicEvolving()
out of curiosity: is there a way to have a kind test comparing java/scala classes annotated with @PublicEvolving
and similar entities in python and failing if there is something annotated in java/scala and not annotated in python and vice versa.
Sure not under this PR, just in general as an idea...
What is the purpose of the change
Currently, the Java APIs use annotations like
@PublicEvolving
and@Experimental
to clearly indicate the stability status of API components. This is super helpful for understanding which parts of the API are stable for production use and which might change in future releases.The Python APIs lack this kind of explicit stability indication. This PR adds a set of decorators, corresponding to the annotations used in the Java APIs, that allow us to decorate classes/functions depending on their stability. These decorators also modify the underlying docstrings of the classes/functions that they decorate, so that the API stability is also reflected in the Sphinx documentation. Additionally, classes/functions that are deprecated output a warning at runtime on their invocation.
The decorators function at both the class and function level. At the function level, they enrich the docstring of the function with an rst block that is rendered as a directive in the Sphinx documentation for that function. At the class level, they enrich the docstring of the class and also the docstrings of the classes public functions, so that, for example, the function documentation for a class annotated as
PublicEvolving()
also contains a directive block informing the user that the class the function is part of is marked as public evolving. The following decorators, and their resulting documentation changes:Decorators
Deprecated
Deprecated
is distinct from the other decorators in that it takes a set of arguments, a mandatorysince
argument and an optionaldetail
argument. These are output as a Sphinx deprecated directive, hence the mandatorysince
argument.Without
detail
:Output in the Sphinx documentation:

With
detail
:Output in the Sphinx documentation:

Experimental
Output in the Sphinx documentation:

Internal
Output in the Sphinx documentation:

PublicEvolving
Output in the Sphinx documentation:

Combining Decorators
The decorators can also be combined. Multiple decorators can be applied to a class/function, and this will output the directives of all the decorators in the documentation for that class/function. Decorators on classes and then on functions in those classes will also combine, so that the decoration from the class is propagated down to the function and combined with the annotations on that function. So, for example, the following code:
Will produce the following on the documentation for the function

ResolvedSchema.get_columns
:Brief change log
PublicEvolving/Deprecated/Internal/Experimental
decorators for Pyflink APIs.Verifying this change
This change added tests and can be verified as follows:
Does this pull request potentially affect one of the following parts:
@Public(Evolving)
: (no)Documentation