Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Adding awkward.semipublic submodule #3152

Merged

Conversation

tcawlfield
Copy link
Collaborator

This should start the ball rolling on a phased approach to resolving 2856, and more generally
provide a place to put functions that are not well-documented and not available in the top-level
awkward namespace, but are still available to external projects in the sense that we promise
to maintain future support in at least minor and patch releases.

I'm adding a new submodule, awkward.semipublic. This is not particularly inspired, but I believe
that a new submodule named something is necessary, in part because of the thirsty behavior in
src/awkward/__init__.py:

from awkward.operations import *
...
__all__ = [x for x in globals() if not x.startswith("_")]

So if we were to sprinkle these semi-public functions into other modules like ak.operations, they would
wind up in the top-level namespace unless we did fancy things like a @semipublic decorator. I don't
think that's worth the effort, even though I also don't particularly like my submodule name. Any other
suggestions?

@jpivarski
Copy link
Member

I don't think they should be organized into a named submodule called "semipublic." It's an attribute of the data, but not an ontological category. (A lot of unrelated things would go into that submodule, which would not make them any easier to find.) Besides, all of the currently semipublic APIs would have to get doubled, since we can't remove the old API without a deprecation cycle, and they're currently in places that are named after their function (e.g. in ak.forms.form or something).

@tcawlfield
Copy link
Collaborator Author

I don't think they should be organized into a named submodule called "semipublic." It's an attribute of the data, but not an ontological category. (A lot of unrelated things would go into that submodule, which would not make them any easier to find.) Besides, all of the currently semipublic APIs would have to get doubled, since we can't remove the old API without a deprecation cycle, and they're currently in places that are named after their function (e.g. in ak.forms.form or something).

Okay, that's a good point. I can make awkward.prettyprint an alias for awkward._prettyprint. That's easy -- import ._prettyprint as prettyprint. But I'm not sure where to put remove_structure. Functionally it's a little bit like the other items in ak.operations, but not exactly. Those all have @high_level_function decorators which serves a more elaborate purpose, so remove_structure would be a unicorn there as well as being difficult to avoid exposing at the top level. I guess we could create awkward.do which only contains remove_structure. No, that's very distasteful, having both a .do and ._do submodule. I wanted to avoid awkward.utils because that's no more meaningful than semipublic. awkward.transforms.remove_structure() maybe?

@jpivarski
Copy link
Member

remove_structure can be in ak.contents.content.

@tcawlfield
Copy link
Collaborator Author

remove_structure can be in ak.contents.content.

Okay, I did these both, more or less.
remove_structure went into ak.contents.remove_structure in order to avoid circular imports.

See tests/test_2856_export_remove_structure_and_prettyprint.py

Copy link
Member

@jpivarski jpivarski left a comment

Choose a reason for hiding this comment

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

This is a good way to organize it. If Ragged needs to be updated as well, then that would be a logical next step after this is merged.

If it passes all the tests, this can be merged.

@tcawlfield tcawlfield merged commit 02858f8 into main Jun 18, 2024
41 checks passed
@tcawlfield tcawlfield deleted the 2856-export-remove_structure-and-prettyprint-for-dask-awkward branch June 18, 2024 20:58
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.

Export remove_structure and prettyprint for dask-awkward
2 participants