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: add copy helpers #1697

Closed
wants to merge 6 commits into from
Closed

refactor: add copy helpers #1697

wants to merge 6 commits into from

Conversation

agoose77
Copy link
Collaborator

No description provided.

@codecov
Copy link

codecov bot commented Sep 14, 2022

Codecov Report

Merging #1697 (5f08a37) into main (e692946) will increase coverage by 0.43%.
The diff coverage is 87.18%.

❗ Current head 5f08a37 differs from pull request most recent head 355995d. Consider uploading reports for the commit 355995d to get more accurate results

Additional details and impacted files
Impacted Files Coverage Δ
src/awkward/_v2/_connect/jax/__init__.py 90.47% <ø> (+1.58%) ⬆️
src/awkward/_v2/config.py 0.00% <0.00%> (ø)
src/awkward/_v2/operations/ak_broadcast_arrays.py 100.00% <ø> (+4.34%) ⬆️
src/awkward/_v2/operations/ak_transform.py 65.51% <ø> (+56.89%) ⬆️
src/awkward/_v2/types/uniontype.py 84.61% <ø> (ø)
src/awkward/_v2/behaviors/categorical.py 73.13% <60.00%> (-8.94%) ⬇️
src/awkward/_v2/forms/form.py 82.95% <68.51%> (-7.80%) ⬇️
src/awkward/_v2/highlevel.py 72.09% <72.72%> (+0.25%) ⬆️
src/awkward/_v2/forms/emptyform.py 76.47% <75.00%> (+1.08%) ⬆️
src/awkward/_v2/forms/unionform.py 73.75% <75.00%> (+0.57%) ⬆️
... and 51 more

@agoose77
Copy link
Collaborator Author

Given that we just don't allow cycles in parameters, this is not required.

@agoose77 agoose77 closed this Sep 14, 2022
@jpivarski
Copy link
Member

Okay, so that was the motivation for this!

This was looking to me like an abstraction of __deepcopy__ from a computational implementation, "deepcopy does such-and-such an algorithm," to a declarative specification: "these attributes are deep and those are shallow, a single algorithm for all layouts uses this fact." I didn't think this abstraction was necessary if it was only for __deepcopy__, but if there was some other reason for it, then maybe it would help to have this information around.

But if your goal was just for __deepcopy__ and you don't have any other operations in mind, then I agree that this is probably overkill.

@jpivarski jpivarski deleted the agoose77/add-copy-helpers branch September 14, 2022 13:10
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.

2 participants