Skip to content

Conversation

@mrzimu
Copy link
Contributor

@mrzimu mrzimu commented Oct 27, 2025

Hi,

This PR may be trivial so far, since the regex pattern I modified is for determinating whether a branch is jagged, which the result value is_jagged is never used at present.

I found this mistake when I use _from_leaves_one to check whether a branch is jagged or not in uproot-custom, and I think I might as well fix this bug now.

My understanding to these patterns is that, when a branch contains arrays, there will be [dim1][dim2]... patterns in title. As long as dimx is not an integer, the branch is jagged.

Expected behavior:

  1. Use _item_any_pattern to find all strings matching the [xxx] pattern.
  2. Use _item_dim_pattern to check if xxx is an integer. If not, set is_jagged to True.

Actual behavior (before fixing):

  1. _item_any_pattern captures all [xxx]-like strings but discards the brackets, only retaining xxx.
  2. _item_dim_pattern requires the full [xxx] pattern to validate xxx, but since the brackets are already missed, the input xxx can never satisfy this pattern.

Copy link
Collaborator

@ariostas ariostas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @mrzimu, you're right!

I was hoping that this might also fix #1498, but it seems like it's a separate thing.

Copy link
Collaborator

@ianna ianna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mrzimu - great ! thanks for fixing it!

@mrzimu
Copy link
Contributor Author

mrzimu commented Oct 27, 2025

Thank you @mrzimu, you're right!

I was hoping that this might also fix #1498, but it seems like it's a separate thing.

It doesn't fix #1498, but they should be a little relevant I guess:

#1498 (comment)

@mrzimu
Copy link
Contributor Author

mrzimu commented Oct 27, 2025

And I have no idea why the pipeline failed...

@ariostas
Copy link
Collaborator

Don't worry about it, it's not your fault. I already fixed it in another PR.

@ianna ianna enabled auto-merge (squash) October 28, 2025 11:49
@ianna ianna merged commit 2751f8a into scikit-hep:main Oct 28, 2025
29 checks passed
@mrzimu mrzimu deleted the mrli/fix_is-jagged-determination branch October 28, 2025 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants