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

refactor: hide L3 API #1983

Merged
merged 25 commits into from
Dec 9, 2022
Merged

refactor: hide L3 API #1983

merged 25 commits into from
Dec 9, 2022

Conversation

agoose77
Copy link
Collaborator

@agoose77 agoose77 commented Dec 8, 2022

  • Adds highlevel, behavior arguments to the ak.XXX reducers
  • Removes layout.XXX reducers
  • Moves layout._reduce to ak._do.reduce
  • Rename layout.layout_equal to Content.is_equal_to
  • Removes unused methods of NumpyArray
  • Replaces NumpyArray.maybe_to_array() with NumpyArray.maybe_to_NumpyArray()
  • Renames Content / Content subclass public method/properties
  • Remove Content.tolist() (but keep high-level implementation)
  • Move validity_error to ak._do.validity_error
  • Rename Content.typetracer with Content.to_typetracer(...)
  • Replace Content.forget_length() with argument to Content.to_typetracer()
  • Rename ak.packed() to ak.to_packed() (same for Content.packed())

📚 The documentation for this PR will be available at https://awkward-array.readthedocs.io/en/agoose77-refactor-l3-l4-split/ once Read the Docs has finished building 🔨

@agoose77 agoose77 changed the title refactor: replace maybe_as_array with maybe_to_NumpyArray refactor: hide L3 API Dec 8, 2022
@codecov
Copy link

codecov bot commented Dec 8, 2022

Codecov Report

Merging #1983 (25ca325) into main (bbc24fa) will decrease coverage by 0.03%.
The diff coverage is 90.11%.

Additional details and impacted files
Impacted Files Coverage Δ
src/awkward/operations/ak_corr.py 84.00% <ø> (ø)
src/awkward/operations/ak_covar.py 85.00% <ø> (ø)
src/awkward/operations/ak_isclose.py 100.00% <ø> (ø)
src/awkward/operations/ak_linear_fit.py 86.53% <ø> (ø)
src/awkward/operations/ak_mean.py 68.00% <ø> (ø)
src/awkward/operations/ak_moment.py 82.35% <ø> (ø)
src/awkward/operations/ak_ptp.py 92.00% <ø> (ø)
src/awkward/operations/ak_softmax.py 100.00% <ø> (ø)
src/awkward/operations/ak_to_buffers.py 90.00% <ø> (ø)
src/awkward/operations/ak_var.py 71.42% <ø> (ø)
... and 50 more

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.

We talked about this on Slack while it was in development, and I am completely in favor of these changes.

@agoose77 agoose77 marked this pull request as ready for review December 9, 2022 00:01
@agoose77
Copy link
Collaborator Author

agoose77 commented Dec 9, 2022

I made the last fixes that follow from the list of changes in #1972. In a couple of places, I think I've made the code worse - NumpyArray._raw is used externally but is private. However, the main priority here is hiding functions; later on we can clean this up.

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.

I checked it over again; all is good. Now I'll merge it.

@@ -336,169 +336,6 @@ def dofunction(link, linelink, shortname, name, astfcn):
dofunction(link, linelink, shortname, toplevel.name, toplevel)


categories = [
Copy link
Member

Choose a reason for hiding this comment

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

I see: categories is completely gone, now.

Comment on lines -663 to +645
contiguous_self = self if self.is_contiguous else self.contiguous()
contiguous_self = self.to_contiguous()
Copy link
Member

Choose a reason for hiding this comment

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

I guess that was superfluous, considering that to_contiguous would return self if already contiguous.

@jpivarski jpivarski merged commit 4e4bb54 into main Dec 9, 2022
@jpivarski jpivarski deleted the agoose77/refactor-l3-l4-split branch December 9, 2022 02:17
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.

None yet

2 participants