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

Update coding-practice-trips-by-week-asset.md #23819

Closed
wants to merge 2 commits into from

Conversation

edsoncezar16
Copy link
Contributor

Adapt the solution to address the requirement of not loading the full data into memory.

Summary & Motivation

The challenge asks to design the code assuming that the whole trips data does not fit into memory but a week of data does. When fetching data from the database, the previous solution did the good job of filtering week data, but then went on to append this data to a initially empty dataframe. That approach would still find the same memory overflow.

How I Tested These Changes

Defined a trips by week asset in the local project following the Dagster University Course and successfully materialized it.

Changelog [New | Bug | Docs]

NOCHANGELOG

Adapt the solution to address the requirement of not loading the full data into memory.
@graphite-app graphite-app bot added the area: dagster-university Related to Dagster University label Aug 22, 2024
@graphite-app graphite-app bot requested a review from erinkcochran87 August 22, 2024 14:01
@cmpadden cmpadden self-requested a review August 22, 2024 15:08
@graphite-app graphite-app bot requested a review from PedramNavid September 28, 2024 20:06
@graphite-app graphite-app bot added the area: docs Related to documentation in general label Sep 28, 2024
@graphite-app graphite-app bot requested review from C00ldudeNoonan and removed request for PedramNavid September 28, 2024 20:06
Copy link

graphite-app bot commented Sep 28, 2024

Graphite Automations

"Label and add CE on all Docs" took an action on this PR • (09/28/24)

2 reviewers were added and 1 label was added to this PR based on Pedram Navid's automation.

Copy link
Contributor

@cmpadden mind taking a look?

@cmpadden cmpadden requested review from dehume and removed request for erinkcochran87 and C00ldudeNoonan December 20, 2024 16:06
@cmpadden
Copy link
Contributor

Apologies that this PR went a bit stale, @edsoncezar16 .

We're going to close the PR, but your suggestion is good. Instead of making the code more complex, we're going to have a follow-up to improve the wording of the lesson itself to hopefully be more clear.

Thanks again!

@cmpadden cmpadden closed this Dec 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: dagster-university Related to Dagster University area: docs Related to documentation in general
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants