Skip to content

feat(typing): Add complete typing to core operations (#3849)#4150

Open
Paramveersingh-S wants to merge 5 commits into
scikit-hep:mainfrom
Paramveersingh-S:fix-ak-flatten-typing
Open

feat(typing): Add complete typing to core operations (#3849)#4150
Paramveersingh-S wants to merge 5 commits into
scikit-hep:mainfrom
Paramveersingh-S:fix-ak-flatten-typing

Conversation

@Paramveersingh-S

Copy link
Copy Markdown

Summary

Fixes issue #13 by adding the missing type hint for the axis parameter in ak.flatten.

Problem

The type hint for axis was absent in src/awkward/operations/ak_flatten.py, which caused static type checkers like Pyright to report errors when ak.flatten was used. The docstring correctly notes it can be None, int, or str, but the signature was missing it.

Solution

Added axis: int | str | None = 1 in the function signature to correctly define the supported types for the axis argument, appeasing static type checkers.

@github-actions github-actions Bot added the type/fix PR title type: fix (set automatically) label Jun 23, 2026

@ikrommyd ikrommyd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We will not be adding a type hint to only one parameter of a function and this does definitely not fix issue #13 as you said as it's not even an issue.
We generally want to add typing properly to the public API (see #3849) so if you wanna contribute to that on a larger scale sure.

@Paramveersingh-S Paramveersingh-S changed the title fix: Add missing type hint for axis parameter in ak.flatten feat(typing): Add complete typing to core operations (#3849) Jun 23, 2026
@github-actions github-actions Bot added type/feat PR title type: feat (set automatically) and removed type/fix PR title type: fix (set automatically) labels Jun 23, 2026
@Paramveersingh-S

Copy link
Copy Markdown
Author

Hi @ikrommyd,

First off, I sincerely apologize for the confusion regarding the #13 reference in my original description—that was an accidental reference to an issue tracking board in my personal portfolio repository, not an actual issue in scikit-hep/awkward! I can see how that was misleading.

You are completely right that adding a type hint to a single parameter is an incomplete fix. I have updated the PR title and pushed new commits that take a much larger step towards #3849. I have now added complete, proper static typing signatures to several core operations in the public API: flatten, drop_none, is_none, where, and concatenate, utilizing the types from awkward._typing.

Hopefully, this serves as a solid foundation for #3849! Please let me know if there are any specific adjustments to the type definitions you'd prefer or if there are other operations you'd like me to tackle in this same PR.

@ikrommyd

Copy link
Copy Markdown
Member

We'd need some internal discussion about how to approach the issue and how we want to group PRs about adding typing.

@TaiSakuma

TaiSakuma commented Jun 23, 2026

Copy link
Copy Markdown
Member

Open PRs related to issue #3980 add summary lines to the docstrings, one line below the function signatures this PR edits. That might cause merge conflicts.

@ikrommyd

Copy link
Copy Markdown
Member

The docstrings are very very loosely related to typing and will not cause merge conflicts. They are only related by the fact that the docstrings describe the type in words but loosely.

@TaiSakuma

Copy link
Copy Markdown
Member

They are next to each other. Merge conflicts can occur if you try to merge two branches that edit two lines next to each other.

@ikrommyd

Copy link
Copy Markdown
Member

Yet the one is changing docstrings while typing changes source code.

@Paramveersingh-S

Copy link
Copy Markdown
Author

We'd need some internal discussion about how to approach the issue and how we want to group PRs about adding typing.

"Thanks for the update! Take all the time you need for the internal discussion on how to best group the typing PRs. I'm happy to hold off on further changes, and if any merge conflicts pop up with #3980 down the line, I'll be more than happy to rebase and resolve them. Just let me know how you'd like to proceed when you're ready!"

@codecov

codecov Bot commented Jun 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.43%. Comparing base (74c0254) to head (8ec6508).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
Files with missing lines Coverage Δ
src/awkward/operations/ak_concatenate.py 85.91% <100.00%> (ø)
src/awkward/operations/ak_drop_none.py 100.00% <100.00%> (ø)
src/awkward/operations/ak_flatten.py 94.73% <100.00%> (+0.09%) ⬆️
src/awkward/operations/ak_is_none.py 100.00% <100.00%> (ø)
src/awkward/operations/ak_where.py 95.74% <100.00%> (+0.09%) ⬆️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type/feat PR title type: feat (set automatically)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants