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

Feat: add missing _like methods to TypeTracer #1505

Merged
merged 11 commits into from
Jun 16, 2022
Merged

Conversation

agoose77
Copy link
Collaborator

@agoose77 agoose77 commented Jun 15, 2022

This PR just implements some of the missing TypeTracer methods to fix #1504

Changes:

  • Remove unset from TypeTracer signatures - we should support explicit None
  • Extend from_array to handle literal sequence of primitives
  • Add _like methods

@agoose77
Copy link
Collaborator Author

agoose77 commented Jun 15, 2022

The modified from_array impl doesn't handle a sequence of non-numpy-friendly value types. We now raise an error for this case, rather than permitting 'O' dtypes.

At this point I think we need to decide how flexible from_array needs to be. Namely:

  • should we handle sequence of mixed Unknown(Scalar|Length|MaybeNone), or only accept numpy-friendly types?
  • do we want to accept scalars here like NumPy does?

My thought is to keep it restricted for now. nplike is an internal API (in the sense that users are not expected to work with it), so we can restrict ourselves to sequence-of-known-scalars.

@codecov
Copy link

codecov bot commented Jun 15, 2022

Codecov Report

Merging #1505 (0698234) into main (5a78305) will increase coverage by 0.03%.
The diff coverage is 86.95%.

Impacted Files Coverage Δ
src/awkward/_v2/_typetracer.py 71.12% <86.95%> (+1.22%) ⬆️

@jpivarski
Copy link
Member

This function creates TypeTracerArrays from NumPy arrays to act as buffers within an Awkward Array, so they need to be the kinds of NumPy arrays that can be buffers in an Awkward Array. Those NumPy arrays can't have dtype=object, which means MaybeNone is off the list, though UnknownScalars and UnknownLength both represent integers, and UnknownScalar says what dtype it will be (useful for making the array). UnknownLength is like a Python int, so 64-bit (maybe 32-bit on Windows?).

Is this PR done? It looks done, and I'd like to make a new release very soon (definitely by tomorrow).

Why did you remove the unset = object()? I created that to make a distinction between a keyword argument that wasn't explicitly set and a keyword argument that may have been set to None. Is there no need for such a distinction? (I can't think of any—I was just being cautious.)

@agoose77
Copy link
Collaborator Author

@jpivarski my concern was if we use it internally for anything: I recall doing nplike.asarray on literals in Awkward at one point.
If we're confident this should only receive "valid" values, then I'm happy with what's here currently.

unset = object() is fine if we need it, but we had lots of if arg is unset which fails for when the user passes in the signature-specified default e.g. None. Not sure if that makes too much sense - I have COVID brain right now 🙃

@jpivarski
Copy link
Member

I'm sorry to hear that—I hope you feel better soon!

I think nplike.asarray might be used on a user-provided input in ak.fill_none (on the value that will fill the array). The first attempt is to make that fill value an Awkward Array, though.

The nplike objects are strictly for our internal use, and we can mandate that internal code always specifies arguments. Maybe that would be too annoying. Anyway, I had been thinking that as we learn how our own code calls these nplike functions, we'd put appropriate defaults in there, one at a time replacing the unset objects.

But that's okay; I think that's minor. Thanks for the new methods and I'll include them in the pre-release!

@jpivarski jpivarski merged commit 0c6ae03 into main Jun 16, 2022
@jpivarski jpivarski deleted the feat-typetracer-missing branch June 16, 2022 19:42
@douglasdavis
Copy link
Contributor

douglasdavis commented Jun 16, 2022

Thanks @agoose77! ones_like and zeros_like are working.

edit: I had originally pointed out a new issue with ak.to_numpy but I think it's actually OK.

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.

ak.* support for typetracer arrays
3 participants