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(containers): remove '.copy' method from ContainerBase. #231

Merged
merged 1 commit into from
Jul 30, 2024

Conversation

ljgray
Copy link
Contributor

@ljgray ljgray commented Mar 20, 2023

This method has been implemented in 'MemDiskGroup' in 'caput.memh5', so there is no need to overwrite it here now.

Relies on radiocosmology/caput#239 to properly implement shared datasets.

@ljgray ljgray requested a review from jrs65 March 21, 2023 00:56
@ljgray ljgray marked this pull request as ready for review March 21, 2023 00:56
@ljgray ljgray force-pushed the ljg/memh5-copy branch 2 times, most recently from 46d8be5 to a975654 Compare April 1, 2023 00:28
@ljgray ljgray requested a review from ketiltrout May 21, 2024 18:26
@ljgray ljgray force-pushed the ljg/memh5-copy branch 3 times, most recently from 614ee37 to 8a0f45c Compare May 22, 2024 20:12
This method has been implemented in 'MemDiskGroup' in 'caput.memh5', so
there is no need to overwrite it here now.
@ljgray
Copy link
Contributor Author

ljgray commented Jul 30, 2024

@ketiltrout This should now be fine after merging radiocosmology/caput#239

Copy link
Member

@ketiltrout ketiltrout left a comment

Choose a reason for hiding this comment

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

I think this looks fine. Certainly, the draco test suite seems sufficient to let us know if this broke something.

@ljgray ljgray merged commit 495d122 into master Jul 30, 2024
4 checks passed
@ljgray ljgray deleted the ljg/memh5-copy branch July 30, 2024 00:57
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