Skip to content
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

Merged
merged 2 commits into from
Sep 12, 2023

Conversation

smackesey
Copy link
Collaborator

@smackesey smackesey commented Jul 17, 2023

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
image
    @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.
image
@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,

image

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.

@smackesey
Copy link
Collaborator Author

smackesey commented Jul 17, 2023

@github-actions
Copy link

github-actions bot commented Jul 17, 2023

Deploy preview for dagster ready!

Preview available at https://dagster-o2adx9lwt-elementl.vercel.app

Direct link to changed pages:

@smackesey smackesey changed the title [docs] Add deprecated/experimental flags and warnings. [docs] Add deprecated/experimental flags and warnings to docs Jul 20, 2023
@smackesey smackesey changed the base branch from sean/upgrade-sphinx to sean/dagster-sphinx July 24, 2023 14:07
@github-actions
Copy link

github-actions bot commented Jul 29, 2023

Deploy preview for dagster-docs ready!

Preview available at https://dagster-docs-kdbtpmgwz-elementl.vercel.app
https://sean-docs-add-flags.dagster.dagster-docs.io

Direct link to changed pages:

Base automatically changed from sean/dagster-sphinx to master September 12, 2023 19:57
@smackesey smackesey marked this pull request as ready for review September 12, 2023 20:02
Copy link
Contributor

@erinkcochran87 erinkcochran87 left a 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:
    Screenshot 2023-09-12 at 4 13 24 PM

@smackesey
Copy link
Collaborator Author

On styling, however, I have a few thoughts:

@hellendag is going to do a followup PR to correct the styling

@smackesey smackesey merged commit 9243d45 into master Sep 12, 2023
2 checks passed
@smackesey smackesey deleted the sean/docs-add-flags branch September 12, 2023 21:43
jmsanders pushed a commit that referenced this pull request Sep 13, 2023
## 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)
zyd14 pushed a commit to zyd14/dagster that referenced this pull request Sep 15, 2023
…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants