-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[docs] Add deprecated/experimental flags and warnings to docs #15311
Conversation
Current dependencies on/for this PR:
This comment was auto-generated by Graphite. |
Deploy preview for dagster ready! Preview available at https://dagster-o2adx9lwt-elementl.vercel.app Direct link to changed pages: |
8de8f30
to
2354769
Compare
81a2394
to
e6d3d87
Compare
2354769
to
10db128
Compare
e6d3d87
to
32fa65d
Compare
10db128
to
29a8dea
Compare
32fa65d
to
8da1352
Compare
29a8dea
to
aa9c1a8
Compare
8da1352
to
0eb452e
Compare
0eb452e
to
a9bed03
Compare
a947be7
to
8a67b5f
Compare
8a67b5f
to
fc94410
Compare
a9bed03
to
bd1af12
Compare
Deploy preview for dagster-docs ready! Preview available at https://dagster-docs-kdbtpmgwz-elementl.vercel.app Direct link to changed pages: |
fc94410
to
a4aee2b
Compare
bd1af12
to
b96cf5a
Compare
b96cf5a
to
eead62e
Compare
6e60026
to
b4fcee2
Compare
eead62e
to
3c985b2
Compare
b4fcee2
to
82c08ab
Compare
3c985b2
to
7377066
Compare
82c08ab
to
7342c0a
Compare
7377066
to
44801b4
Compare
7342c0a
to
71bbdc0
Compare
44801b4
to
819d58c
Compare
71bbdc0
to
1086428
Compare
819d58c
to
464c88d
Compare
1086428
to
7bf84f1
Compare
464c88d
to
1b08e0b
Compare
7bf84f1
to
349d11a
Compare
1b08e0b
to
aa00de1
Compare
aa00de1
to
164a809
Compare
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.
Overall I like this a lot! It's so much easier to identify non-stable/unavailable things now. No qualms with the approach or how it's implemented.
On styling, however, I have a few thoughts:
- Nitpick - The box containing the the API-level deprecation (the first example in the PR description) doesn't line up with the text that follows it - it's slightly more to the right. It'd be super if the box/callout and text could be in line with each other.
- The UI has styles for labels similar to these - should we use that styling (or similar) to be consistent? Ex:
@hellendag is going to do a followup PR to correct the styling |
9ea0703
to
9270312
Compare
## Summary & Motivation #15077 introduced special rendering of "Deprecated" tags in the docs. This PR expands that effort by leveraging the downstack consolidation of deprecated/experimental warnings into the annotations layer (#15302, #15305). With the decorator APIs for warnings, one declaration is sufficient to specify information for both the docs and a runtime warning. This PR makes `@deprecated`, `@deprecated_param`, `@experimental`, and `@experimental_param` render a special tag and associated warning text in the docs. Examples: ``` @public @deprecated( breaking_version="2.0", additional_warn_text="Setting this property no longer has any effect.", ) @Property def environment_vars(self) -> Mapping[str, str]: """Mapping[str, str]: Environment variables to export to the cron schedule.""" return self._environment_vars ``` <img width="1197" alt="image" src="https://github.com/dagster-io/dagster/assets/1531373/8ce5483b-6ea3-4aba-a793-18a45f87081e"> ``` @public @staticmethod @experimental def table( records: Sequence[TableRecord], schema: Optional[TableSchema] = None ) -> "TableMetadataValue": """Static constructor for a metadata value wrapping arbitrary tabular data as :py:class:`TableMetadataValue`. Can be used as the value type for the `metadata` parameter for supported events. ``` <img width="1191" alt="image" src="https://github.com/dagster-io/dagster/assets/1531373/a4c75e07-8a18-4db9-8e59-6dfadda63d3f"> ``` @experimental_param(param="resource_defs") @experimental_param(param="io_manager_def") @experimental_param(param="auto_materialize_policy") @deprecated_param( param="non_argument_deps", breaking_version="2.0.0", additional_warn_text="use `deps` instead." ) def asset( compute_fn: Optional[Callable] = None, *, name: Optional[str] = None, key_prefix: Optional[CoercibleToAssetKeyPrefix] = None, ins: Optional[Mapping[str, AssetIn]] = None, deps: Optional[Sequence[Union[CoercibleToAssetKey, AssetsDefinition, SourceAsset]]] = None, ``` <img width="1191" alt="image" src="https://github.com/dagster-io/dagster/assets/1531373/b859d74f-baeb-49af-9050-ec4084569292"> As a result of this, it is no longer necessary to describe an entity as deprecated/experimental in its docstring-- that info will come from the decorator. So this PR also removes some of this info from docstrings. The bulk of the code for this is in the sphinx extension, but I also added some CSS and a new color for the "experimental" tags. Happy to go with other styling suggestions. ## How I Tested These Changes Built docs. (cherry picked from commit 9243d45)
…r-io#15311) ## Summary & Motivation dagster-io#15077 introduced special rendering of "Deprecated" tags in the docs. This PR expands that effort by leveraging the downstack consolidation of deprecated/experimental warnings into the annotations layer (dagster-io#15302, dagster-io#15305). With the decorator APIs for warnings, one declaration is sufficient to specify information for both the docs and a runtime warning. This PR makes `@deprecated`, `@deprecated_param`, `@experimental`, and `@experimental_param` render a special tag and associated warning text in the docs. Examples: ``` @public @deprecated( breaking_version="2.0", additional_warn_text="Setting this property no longer has any effect.", ) @Property def environment_vars(self) -> Mapping[str, str]: """Mapping[str, str]: Environment variables to export to the cron schedule.""" return self._environment_vars ``` <img width="1197" alt="image" src="https://github.com/dagster-io/dagster/assets/1531373/8ce5483b-6ea3-4aba-a793-18a45f87081e"> ``` @public @staticmethod @experimental def table( records: Sequence[TableRecord], schema: Optional[TableSchema] = None ) -> "TableMetadataValue": """Static constructor for a metadata value wrapping arbitrary tabular data as :py:class:`TableMetadataValue`. Can be used as the value type for the `metadata` parameter for supported events. ``` <img width="1191" alt="image" src="https://github.com/dagster-io/dagster/assets/1531373/a4c75e07-8a18-4db9-8e59-6dfadda63d3f"> ``` @experimental_param(param="resource_defs") @experimental_param(param="io_manager_def") @experimental_param(param="auto_materialize_policy") @deprecated_param( param="non_argument_deps", breaking_version="2.0.0", additional_warn_text="use `deps` instead." ) def asset( compute_fn: Optional[Callable] = None, *, name: Optional[str] = None, key_prefix: Optional[CoercibleToAssetKeyPrefix] = None, ins: Optional[Mapping[str, AssetIn]] = None, deps: Optional[Sequence[Union[CoercibleToAssetKey, AssetsDefinition, SourceAsset]]] = None, ``` <img width="1191" alt="image" src="https://github.com/dagster-io/dagster/assets/1531373/b859d74f-baeb-49af-9050-ec4084569292"> As a result of this, it is no longer necessary to describe an entity as deprecated/experimental in its docstring-- that info will come from the decorator. So this PR also removes some of this info from docstrings. The bulk of the code for this is in the sphinx extension, but I also added some CSS and a new color for the "experimental" tags. Happy to go with other styling suggestions. ## How I Tested These Changes Built docs.
Summary & Motivation
#15077 introduced special rendering of "Deprecated" tags in the docs. This PR expands that effort by leveraging the downstack consolidation of deprecated/experimental warnings into the annotations layer (#15302, #15305). With the decorator APIs for warnings, one declaration is sufficient to specify information for both the docs and a runtime warning.
This PR makes
@deprecated
,@deprecated_param
,@experimental
, and@experimental_param
render a special tag and associated warning text in the docs. Examples:As a result of this, it is no longer necessary to describe an entity as deprecated/experimental in its docstring-- that info will come from the decorator. So this PR also removes some of this info from docstrings.
The bulk of the code for this is in the sphinx extension, but I also added some CSS and a new color for the "experimental" tags. Happy to go with other styling suggestions.
How I Tested These Changes
Built docs.