Skip to content

feat: support optional flag in from_ extractors#1552

Open
mgdemers wants to merge 1 commit intoapache:mainfrom
mgdemers:feat/mgdemers/support_optional_extractors
Open

feat: support optional flag in from_ extractors#1552
mgdemers wants to merge 1 commit intoapache:mainfrom
mgdemers:feat/mgdemers/support_optional_extractors

Conversation

@mgdemers
Copy link
Copy Markdown

@mgdemers mgdemers commented Apr 19, 2026

Support optional from_ extractors when DAG modules' targets do not reference an extractor.

Changes

  • Added optional flag to ExtractorFactory
  • When no target node is identified during generate_nodes, return an empty list instead of raising ValueError when flag is set.
  • Add unit test for an optional extractor not referenced by a DAG module

How I tested this

  • Unit tests

Notes

Discussion in #1551

Checklist

  • PR has an informative and human-readable title (this will be pulled into the release notes)
  • Changes are limited to a single goal (no scope creep)
  • Code passed the pre-commit check & code is left cleaner/nicer than when first encountered.
  • Any change in functionality is tested
  • New functions are documented (with a description, list of inputs, and expected output)
  • Placeholder code is flagged / future TODOs are captured in comments
  • Project documentation has been updated if adding/changing functionality.

@mgdemers mgdemers force-pushed the feat/mgdemers/support_optional_extractors branch from 17d214d to df408f2 Compare April 19, 2026 21:49

:param target: Parameter, into which we're loading the data
:param loaders: A list of data loaders that are viable candidates, given the key after `from_`
:param optional: Whether missing targets should raise exceptions or skip being loaded.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add a bit more here.

Suggested change
:param optional: Whether missing targets should raise exceptions or skip being loaded.
:param optional: Whether missing targets should raise exceptions or skip being loaded. Optional=True will mean that if the target does not exist in your graph, we skip try to inject this into the graph.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added - I also added a note on the materialization docs about specifying optional data loaders.

Copy link
Copy Markdown
Contributor

@skrawcz skrawcz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good. can you update the materialization docs too? A comment or example snippet should suffice.

@mgdemers mgdemers force-pushed the feat/mgdemers/support_optional_extractors branch from df408f2 to f8c0a51 Compare April 25, 2026 19:47
@mgdemers mgdemers marked this pull request as ready for review April 25, 2026 19:49
* Add optional flag to ExtractorFactory to skip data loader when target
  was not found rather than raise exceptions.
* Add unit tests for optional and non-optional extractors.
* Add more detailed comments and documentation note.
@mgdemers mgdemers force-pushed the feat/mgdemers/support_optional_extractors branch from f8c0a51 to 00d444d Compare April 25, 2026 19:54
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