unfortunately @jrueb has left academia for industry, unless he wants to review out of the kindness of his heart.

I'm putting in a deprecation for ~6 months from now over in #997.

@lgray Is the plan to keep the public delta_phi and delta_r functions as is? I suggest you do because there are some cases where a user defined "eta" instead of the .eta attribute is needed

In the first phase of this, yes we'll keep those functions for backwards compatibility.

In the long run, when we remove nanoevents.methods.vector, vector supplies deltaR and deltaphi and folks should migrate to use what is already supplied. You can see we now just use those functions within their underscore equivalents.

@nsmith- tests fail after appeasing linter :-(

Yes, I added tests and am now working to improve them

Ah, gotcha - yeah putting this together with scikit-hep/vector#413 I figured you may be messing around.

I think one place where we could benefit a lot from a thorough look-over by @Saransh-cpp is to make sure we're not unintentionally clobbering any of the vector interface with parts of nanoevents.vector, and likewise @nsmith- if there's anything we can do to make the delta between skh-vector-backed nanoevents.vector closer to the original implementation we should try to smooth all that out.

@nsmith- @Saransh-cpp from the looks of it we are now close to being able to merge this change. Please provide reasons not to do so yet, or any further tests that must be done before proceeding.

This fails on a real user analysis with:

File "/Users/lgray/coffea-dev/ewkcoffea/ewkcoffea/modules/selection_wwz.py", line 334, in get_mt2
    rest_WW = w_lep0 + w_lep1 + misspart
...
TypeError: no numpy.add overloads for custom types: Candidate, PtEtaPhiMLorentzVector

Please update and harden tests before we merge. This appears to be on the coffea side of things.

@Saransh-cpp @nsmith-

Ah, this is going to be an issue. We have a class hierarchy that in principle would all need dispatch updates, so fixing it for Candidate just means that now it will fail for Electron or Jet. I'm kind of puzzled why awkward cannot do the type resolution automatically.

So #1063 is just pushing the issue one level down.

Oh yes, I missed that, thanks! I don't think there is a better way to do this? Should I add dispatch updates for Electron and Jet in a similar way?

More tests are failing, and I think there are more classes for which we'll have to do the same - Particle, Muon, MasslessParticle, Photon, Track, Tower, Vertex, MissingET - everything inheriting the coffee vector classes.

@jpivarski is there a way to have awkward dispatch using the method resolution order of the mixin types, rather than having to register every subtype explicitly?

The ak.behavior → Python class object mapping (that is, through the __record__ parameter name) has to map to the concrete classes, but those concrete classes can share superclasses. If we're talking about getting methods and properties on class instances, those methods and properties can be inherited onto the concrete class, but the concrete class must be the one that ak.behavior points to.

For ufunc and binary operator overloading, it's still the concrete classes that have to be named, but those a free functions, rather than methods and properties, so there isn't an easy way for inheritance to work. If you want to make np.add call function f for all subclasses of T, you need to put signatures with each multiple dispatch combination into the ak.behavior. That's why Vector has for loops like this:

https://github.com/scikit-hep/vector/blob/06a733784d8df77d2fe4ad0f87051f725fd6a831/src/vector/backends/awkward.py#L1519-L1546

@agoose77 and I brought this up several times—how to describe inheritance in ak.behavior so that a single line

ak.behavior[np.add, "Vector", "Vector"] = lambda v1, v2: v1.add(v2)

would effectively do

for left in [names of all subclasses of Vector]:
    for right in [names of all subclasses of Vector]:
        ak.behavior[np.add, left, right] = lambda v1, v2: v1.add(v2)

The problem is that the interface of ak.behavior gets more complicated: we'd have to tell it what the names of all subclasses of some common class is independently of Python's own class inheritance mechanism—having two class inheritance mechanisms, Python's and Awkward Array's, which can get out of sync, could lead to an amazingly messy situation. These names, "Vector", "Vector2D", etc., are distinct from Python class objects because parameters have to be JSON to be serialized in Forms, passed from C++, to and from Julia, etc.

Having to build these things with for loops is cumbersome, but I'm afraid that having two parallel inheritance mechanisms would be worse in practice.

Thank you for the explanation! I will add the loops for every vector class in coffee. Additionally, should coffea also have distinct and not the class names in the behavior dict?

Additionally, should coffea also have distinct and not the class names in the behavior dict?

I'm not sure what you mean. Is it the distinction between putting behaviors in a global ak.behaviors versus putting them in each ak.Array.behaviors?

The fact that Coffea used ak.Array.behaviors was great for debugging that feature, and ak.Array should have that feature. However, when I present Vector, I always do vector.register_awkward() once globally after import because then I don't have to show special constructors—Awkward Arrays with Vectors are just ordinary Awkward Arrays. Coffea hasn't had this problem because the data come from files—there has to be a special constructor anyway.

But maybe some of Coffea's metadata handling costs come from passing behaviors from one ak.Array to another. (The same costs would have been present in Coffea 0.7, but maybe they're exacerbated by Dask.) If the behaviors are being passed around as an unexamined dict, then there's no appreciable cost, but if the dicts need to be copied or manipulated, it might add up.

Maybe I'm misunderstanding @Saransh-cpp's question, but either way, that might be something to consider: switching to coffea.register_awkward() (if not doing it automatically with import coffea).