-
Notifications
You must be signed in to change notification settings - Fork 1.7k
[docs] updates docs to favor @dataclass for component authoring #28900
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] updates docs to favor @dataclass for component authoring #28900
Conversation
...es/docs_snippets/docs_snippets/guides/components/shell-script-component/with-custom-scope.py
Outdated
Show resolved
Hide resolved
...ries/dagster-dg/dagster_dg/templates/COMPONENT_TYPE/COMPONENT_TYPE_NAME_PLACEHOLDER.py.jinja
Outdated
Show resolved
Hide resolved
...ocs_snippets/docs_snippets/guides/components/shell-script-component/2-shell-command-empty.py
Outdated
Show resolved
Hide resolved
Deploy preview for dagster-docs ready! Preview available at https://dagster-docs-2vune4kd2-elementl.vercel.app Direct link to changed pages: |
should we add a little bit of docs explanation for the dataclass usage? like make it clear that dataclass usage is optional but gets rid of boilerplate |
So I think we should scaffold |
@@ -9,17 +9,12 @@ | |||
import dagster as dg | |||
|
|||
|
|||
@dataclass | |||
@dataclass(frozen=True) |
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.
although we use frozen
internally our scaffolding does not product it so it might just be confusing.
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.
Rebased your changes, and removed the frozen=True
.
@OwenKephart I also included a note that this is opt-in, with the ability to pass --no-dataclass
.
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.
Let's drop frozen and I think you should rebase on my changes
Deploy preview for dagit-storybook ready! ✅ Preview Built with commit 9b1b985. |
Deploy preview for dagit-core-storybook ready! ✅ Preview Built with commit 9b1b985. |
9b1b985
to
97c5683
Compare
...es/labs/components/creating-new-component-types/creating-and-registering-a-component-type.md
Outdated
Show resolved
Hide resolved
45cf6e1
to
81df8b0
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.
👍🏻
Summary & Motivation
Current docs promote
__init__
methods for defining YAML schema. This updates the tutorial to use data classes.How I Tested These Changes
Changelog
NOCHANGELOG