C++ refactoring: ak.with_name

[ioanaif]

No description provided.

[ioanaif]

@jpivarski Since I believe we changed the printing layout, should the following two test also be updated?:

        behavior = {}
        behavior["__typestr__", "Point"] = "P"
        behavior["Point"] = Point
        array = ak._v2.highlevel.Array(
            [
                [{"x": 1, "y": [1.1]}, {"x": 2, "y": [2.0, 0.2]}],
                [],
                [{"x": 3, "y": [3.0, 0.3, 3.3]}],
            ],
            with_name="Point",
            behavior=behavior,
            check_valid=True,
        )
       assert repr(array[0, 0]) == "<1 [1.1]>"

E assert "<Record {x: 1, y: [1.1]} type='P'>" == '<1 [1.1]>'

       one = ak._v2.highlevel.Array([[{"x": 1}], [], [{"x": 2}]], with_name="One")
       two = ak._v2.highlevel.Array([[{"x": 1.1}], [], [{"x": 2.2}]], with_name="Two")
       assert (
           str(ak._v2.operations.structure.with_name(ak.operations.structure.concatenate([one, two], axis=1), "All").type)
           == '3 * var * All["x": float64]'
       )

E assert '3 * var * var * string' == '3 * var * All["x": float64]'

[codecov]

Codecov Report

Merging #1233 (6a1eb76) into main (0a0e9be) will decrease coverage by 0.74%.
The diff coverage is 82.49%.

Impacted Files	Coverage Δ
src/awkward/_v2/contents/bitmaskedarray.py	64.45% <ø> (-0.79%)	⬇️
src/awkward/_v2/contents/bytemaskedarray.py	82.81% <ø> (+3.66%)	⬆️
src/awkward/_v2/contents/emptyarray.py	69.54% <ø> (ø)
src/awkward/_v2/contents/indexedarray.py	60.74% <0.00%> (+0.98%)	⬆️
src/awkward/_v2/contents/content.py	81.57% <28.57%> (-1.62%)	⬇️
src/awkward/_v2/_typetracer.py	68.94% <33.33%> (-0.25%)	⬇️
src/awkward/_v2/highlevel.py	65.21% <60.00%> (+0.22%)	⬆️
...c/awkward/_v2/operations/reducers/ak_linear_fit.py	85.71% <66.66%> (-1.52%)	⬇️
src/awkward/_v2/operations/reducers/ak_all.py	91.66% <75.00%> (-8.34%)	⬇️
src/awkward/_v2/operations/reducers/ak_any.py	91.66% <75.00%> (-8.34%)	⬇️
... and 42 more

[jpivarski]

Since I believe we changed the printing layout, should the following two test also be updated?

In the first one, the custom behavior is supposed to override the ak._v2.Array.__repr__. Here's the whole test:

https://github.com/scikit-hep/awkward-1.0/blob/4d77f519b6aebb6555fea3c73c73a1fe62b41a6f/tests/test_0049-distinguish-record-and-recordarray-behaviors.py#L10-L33

The fact that the Point class has a __repr__ should matter. I'm not surprised that Point.__repr__ is getting ignored here because ak.Array.__repr__ was completely rewritten from this implementation to this implementation because of #838. The new implementation probably does not include a check for overridden __repr__.

Maybe just skip this one and we'll get back to it in the final clean-up phase.

[jpivarski]

       one = ak._v2.highlevel.Array([[{"x": 1}], [], [{"x": 2}]], with_name="One")
       two = ak._v2.highlevel.Array([[{"x": 1.1}], [], [{"x": 2.2}]], with_name="Two")
       assert (
           str(ak._v2.operations.structure.with_name(ak.operations.structure.concatenate([one, two], axis=1), "All").type)
           == '3 * var * All["x": float64]'
       )

E assert '3 * var * var * string' == '3 * var * All["x": float64]'

As for this one, it looks like something is going terribly wrong. We know that record names are being used in type strings already:

>>> ak._v2.highlevel.Array(
...     ak._v2.contents.RecordArray([
...         ak._v2.contents.NumpyArray([1, 2, 3])],
...         ["field"],
...         parameters={"__record__": "SeeThisName"},
...     )
... )
<Array [{field: 1}, {field: 2}, {field: 3}] type='3 * SeeThisName[field: in...'>

and that's what ak._v2.operations.structure.with_name is supposed to do: add the "__record__" parameter.

But you're getting a type string that says "3 * var * var * string"; it's probably the array that's wrong and not just the type string. Somewhere along the line, some argument was probably misunderstood and then it got iterated over as a string, maybe even one character per string (because of the var * var).

[jpivarski]

As described above, the tests/v2/test_0049-distinguish-record-and-recordarray-behaviors.py test can be skipped with a note to come back later and implement overridden __repr__ in _prettyprint.valuestr.

Something wrong is happening in tests/v2/test_0788-indexedarray-carrying-recordarray-parameters.py that will require more investigation.

tests/v2/test_1233-ak-with_name.py should be a clear-cut case. I'm assuming that none of the test failures are due to that one, but I haven't checked.

[ioanaif]

As described above, the tests/v2/test_0049-distinguish-record-and-recordarray-behaviors.py test can be skipped with a note to come back later and implement overridden __repr__ in _prettyprint.valuestr.

Something wrong is happening in tests/v2/test_0788-indexedarray-carrying-recordarray-parameters.py that will require more investigation.

tests/v2/test_1233-ak-with_name.py should be a clear-cut case. I'm assuming that none of the test failures are due to that one, but I haven't checked.

Thank you for the clarifications!
I believe everything is fixed now. For tests/v2/test_0788-indexedarray-carrying-recordarray-parameters.p there was a bug where the parameters were not getting passed along.

[jpivarski]

For tests/v2/test_0788-indexedarray-carrying-recordarray-parameters.p there was a bug where the parameters were not getting passed along.

That makes sense, actually. Now that you've reminded me of that issue, is one that we should check when something like this happens. (Arrays containing strings may be an easy way to catch "parameters not being passed" bugs.)

[jpivarski]

Great! This is all set, so I'll merge it now and it can get included in 1.8.0rc2.