fix: support non-hashable values in parameters_union
#2651
Conversation
Codecov Report
Additional details and impacted files
|
|
@agoose77 I pip installed from this branch and my reproducer on dask-contrib/dask-awkward#340 and my code on https://github.com/iasonkrom/egamma-tnp work fine now. Looks good from my perspective but that's up to you to judge. Thanks! |
src/awkward/_parameters.py
Outdated
| has_no_exclusions = len(exclude) == 0 | ||
| has_exclusions = bool(exclude) |
There was a problem hiding this comment.
bool(x) is a rather brief way to say len(x) != 0. It works through Python's truthiness, but I would prefer it if Python required us to be more explicit. Nevertheless, this does look like something a flake8-style linter would suggest. :)
There was a problem hiding this comment.
Hehe, I wondered if you'd mind this given that linters generally prefer it. Here, I want to mean both x is not None or len(x) != 0
There was a problem hiding this comment.
I prefer x is not None and also len(x) != 0 over bool(x), but now that we're talking
x is not None and len(x) != 0Well, that is a lot more characters than
bool(x)I guess it's okay. You know, the fact that I didn't realize that x might be None by reading the code is an argument in favor of the long expression. I know that what type hints are for, but the type hint is far away and long-time Python users have gotten used to looking for clues about a variable's type from nearby context. So I still kinda prefer the long expression, as long as black doesn't split it up onto multiple lines. (I don't think it would.)
Fixes dask-contrib/dask-awkward#340