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.

Sign up for GitHub

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

Jump to bottom

Fixes Annotated use as input type. Fixes #554 #557

Merged
merged 2 commits into from
Nov 25, 2023
Merged

Fixes Annotated use as input type. Fixes #554 #557

merged 2 commits into from
Nov 25, 2023

Conversation

skrawcz
Copy link
Collaborator

@skrawcz skrawcz commented Nov 25, 2023

Turns out we were not inspecting the input types like we do the output types. This change fixes that and adds a unit test to ensure the types are what we think they should be.

Changes

  • node constructor pulling types from a function

How I tested this

  • locally with unit tests

Notes

Code from #554 now passes.

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.

@skrawcz
Fixes Annotated use as input type. Fixes #554
1dd59f2 
Turns out we were not inspecting the input types like we do the output types.
This change fixes that and adds a unit test to ensure the types are what
we think they should be.
@skrawcz skrawcz linked an issue Nov 25, 2023 that may be closed by this pull request
Unable to correctly use Annotated NumPy types #554
Closed
@DAGWorks-Inc DAGWorks-Inc deleted a comment from sweep-ai bot Nov 25, 2023
elijahbenizzy
elijahbenizzy approved these changes Nov 25, 2023
View reviewed changes
Copy link
Collaborator

@elijahbenizzy elijahbenizzy left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM once you fix the tests :)

@elijahbenizzy elijahbenizzy self-requested a review November 25, 2023 06:01
@skrawcz
Fixes spark unit test with annotated
11f98eb 
We didn't take into account that annotated would be passed
all the way through in the spark code. This fixes that and adds
a unit test for it.
@skrawcz skrawcz merged commit 5830ef0 into main Nov 25, 2023
@skrawcz skrawcz deleted the fix_handling_annotation branch November 25, 2023 06:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@elijahbenizzy elijahbenizzy Awaiting requested review from elijahbenizzy

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Unable to correctly use Annotated NumPy types
2 participants
@skrawcz @elijahbenizzy