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

Reference Dagger.EAGER_THUNK_STREAMS explicitly #484

Merged
merged 2 commits into from
Mar 14, 2024

Conversation

JamesWrigley
Copy link
Collaborator

A lot of tests failed when I was running them locally because that variable is defined in the Dagger module rather than Dagger.Sch. Though a lot still fail for me with this error:

  Got exception outside of a @test                                                                                                                                                                                                             
  On worker 2:                                                                                                                                                                                                                                 
  Tuple field type cannot be Union{}

I think we're running into JuliaLang/julia#52385 😬 I'll try to look into that in a few days.

(P.S.: I configured the PR to merge into #463 instead of master)

return_type() is kinda broken in v1.10, see:
JuliaLang/julia#52385

In any case Base.promote_op() is the official public API for this operation so
we should use it anyway.
@JamesWrigley
Copy link
Collaborator Author

I think this should be fixed in 73c62dc, where I changed the code to use Base.promote_op(). I'm not an expert with that part of Julia though so I may be mistaken.

@jpsamaroo jpsamaroo merged commit b4f41c8 into JuliaParallel:jps/stream2 Mar 14, 2024
5 of 11 checks passed
@jpsamaroo
Copy link
Member

Thanks, this avoids the heartburn I feel when using Base._return_type 😄

@JamesWrigley JamesWrigley deleted the undefined-variable branch March 14, 2024 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants