feat: New features to PackedSelection

[ikrommyd]

Starting a pull request to merge the new features of PackedSelection intto the master branch.
This isn't 100% complete yet as slightly more testing and actual writing of some more unittests is required

[lgray]

Small comment - for better granularity can you make the tests a bit shorter and more focused? Right now one test will fail and we won't know what's wrong without digging in!

[ikrommyd]

Small comment - for better granularity can you make the tests a bit shorter and more focused? Right now one test will fail and we won't know what's wrong without digging in!

In bd37f9f, I split the tests into "basic", "nminusone" and "cutflow" tests.

[lgray]

@iasonkrom just so the tests for your stuff are really exercising a realistic workflow (i.e. reading from a file), as we have found that to be a sticking point. Could you please add some tests that do so, so we aren't blindsided in the future? It's OK if the tests fail for, you can mark them with a pytest.xfail or test for the exception/failure to happen.

[ikrommyd]

@lgray the "nminusone" and "cutflow" tests do use the nano_dy samples which I read once at the top of the test file to avoid reading in every test function. However for the "dak" version of the tests I currently use from_awkward because of the issues in dask-contrib/dask-awkward#256. I could convert those to xfail tests if you want.

[lgray]

Yes, please do - the from_awkward to get your data is quite unrealistic as we discussed.

[lgray]

@iasonkrom can you rebase this to master?

[lgray]

You are having a bad problem and will not go to space today.

[ikrommyd]

@lgray Yeah I accidentally pressed "discard commits" on github and this closed the PR automatically because this synced my branch with master. Is it possible to reopen it?

[lgray]

ditto about partitions as opposed to checking length of arrays.

[lgray]

The tutorial notebook is comprehensive, nice! :-)

[ikrommyd]

@lgray Any idea how to get raising incompatible partitions to work since compatible_partitions doesn't detect incompatiability if var is sliced as I told you in slack dms?

        if self._delayed_mode:
            for name, var in vars.items():
                if not compatible_partitions(var, self._masks[0]):
                    raise IncompatiblePartitions("plot_vars", var, self._masks[0])

[ikrommyd]

And I can't find I way to get rid of this last len call that I want to avoid.

            h.fill(
                dask_awkward.from_awkward(awkward.Array([0]), 1),
                weight=dask_awkward.from_awkward(awkward.Array([len(weight)]), 1),
            )

I basically just want to fill a delayed histogram with zeros, len(weight) times but the layouts just don't work if I just do dak.count

[lgray]

what's the error you get if you try to do dak.count ?

[lgray]

Also why doesn't h.fill(dak.zeros_like(weight), weight=1.0) work?

[lgray]

for incompatible partitions can you print out the dask arrays you're getting at that stage, since we know it fails later when trying to apply a mask for exactly the same reason.

[ikrommyd]

h.fill(dak.zeros_like(weight), weight=1.0)

yup this works. I'm just stupid and forgot about it.

[ikrommyd]

for incompatible partitions can you print out the dask arrays you're getting at that stage, since we know it fails later when trying to apply a mask for exactly the same reason.

not anymore, because this was just replaced by the h.fill(dak.zeros_like(weight), weight=1.0). This was the line that was raising the error at _getitem_outer_bool_or_int_lazy_array

[lgray]

oh, fun.

[lgray]

that cannot possibly be that line does not fill anything with a mask?

[ikrommyd]

wait maybe I'm just missing something. It's just that the only test failure was "did not raise incmpatible error" after I changed this line.

[lgray]

Honestly for this I'm willing to save it for a follow-up PR to give some time to think about it correctly.

[lgray]

Oh, and I mean for the case that should raise, when it is testing for partition compatibility can you just print out the dask_awkward.Array's and we take a look at them?

[ikrommyd]

Oh you mean to find the bug? If yes then I'll create a reproducer.
What do I do with pytests though that now fail due to not raising incompatible partitions? Just skip?

[lgray]

yeah just comment out that part of the test for now and let's take our time to do it the right way.

[ikrommyd]

I think I finalized this for now. I believe you can just check today's changes and just merge.