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

Make matroid copy and deepcopy simply return self #37670

Merged
merged 4 commits into from
Apr 12, 2024
Merged

Conversation

gmou3
Copy link
Contributor

@gmou3 gmou3 commented Mar 25, 2024

I abstracted all copy/deepcopy subclass implementations to a generic one in the main matroid class (in matroid.pyx).
I am unsure as to whether something of value is lost with this change (e.g. relating to the manual memo passing in deepcopy).

This follows a recommendation by @mkoeppe made in #37340 (comment).

⌛ Dependencies

Copy link
Contributor

@mkoeppe mkoeppe left a comment

Choose a reason for hiding this comment

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

LGTM.

gmou3 added a commit to gmou3/sage that referenced this pull request Mar 26, 2024
TODO: add FlatsMatroid copy tests in matroid.pyx after sagemath#37670
@vbraun
Copy link
Member

vbraun commented Mar 31, 2024

merge conflict

@gmou3
Copy link
Contributor Author

gmou3 commented Apr 1, 2024

I just tried and it got merged cleanly.

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 1, 2024

This "merge conflict" message means that there was a conflict when Volker tried to merge it into his integration branch.

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 1, 2024

There is a conflict with #37667; if you want, you can merge it here to resolve the conflict.

@gmou3
Copy link
Contributor Author

gmou3 commented Apr 1, 2024

Got it. Thanks!

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 1, 2024

Best to add - Depends on #37667 to the PR description

Copy link

github-actions bot commented Apr 1, 2024

Documentation preview for this PR (built with commit 1d1825f; changes) is ready! 🎉

@vbraun vbraun merged commit 332a9bb into sagemath:develop Apr 12, 2024
13 of 14 checks passed
@mkoeppe mkoeppe added this to the sage-10.4 milestone Apr 12, 2024
@gmou3 gmou3 deleted the copy branch April 12, 2024 23:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants