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

ak.zip does not accept record objects #3023

Closed
agoose77 opened this issue Feb 13, 2024 · 2 comments
Closed

ak.zip does not accept record objects #3023

agoose77 opened this issue Feb 13, 2024 · 2 comments
Labels
bug The problem described is something that must be fixed

Comments

@agoose77
Copy link
Collaborator

Version of Awkward Array

main

Description and code to reproduce

Introduced by #2757

@agoose77 agoose77 added bug (unverified) The problem described would be a bug, but needs to be triaged bug The problem described is something that must be fixed and removed bug (unverified) The problem described would be a bug, but needs to be triaged labels Feb 13, 2024
@jpivarski
Copy link
Member

Copied from scikit-hep/fastjet#268 (comment):

I just saw your note, @agoose77: you consider it a regression but I don't! I don't think zipping over a record makes sense, since zipping is something that applies to collections. The same applies to the numerical scalars (msoftdrop, etc.).

  • If all fields of the ak.zip are collections (of equal length), then that's the normal case, we know what to do.
  • If some fields of the ak.zip are scalars (either records or numbers) and some are collections, then ak.zip should broadcast the scalars to the collections in order to be consistent, but that's not likely the user's intention. In this case, I thought it was pretty clear that only one overall value of msoftdrop, etc. is desired, not one per constituent.
  • If all fields are scalars, ak.zip doesn't have any clear meaning. If you want a single record to be built from those scalars, use the ak.Record constructor (as in the second example above).

So ak.zip should at least refuse the all-scalars case and should probably exclude the some-scalars case. (Although I suppose that it's not excluding all-numbers right now.)

@agoose77
Copy link
Collaborator Author

I like this rationale! Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The problem described is something that must be fixed
Projects
None yet
Development

No branches or pull requests

2 participants