-
-
Notifications
You must be signed in to change notification settings - Fork 401
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Deprecate rename_vertices parameter in disjoint_union #35907
Conversation
You cannot simply remove it. You must first deprecate it. So set its default value to if rename_vertices is not None:
from sage.misc.superseded import deprecation
deprecation(35907, 'the "rename_vertices" argument is deprecated') cc-ing @jhpalmieri in case you have an opinion on this argument. |
Since the variable has no effect, I don't think it's that important to deprecate it. I don't object to deprecating it, but it's not a big deal to me. |
Thanks. Policy is to deprecate it just case someone has code that is using it. Anyways, I wasn't sure if you had a reason this argument should actually be implemented (and hence, would be a bug instead). |
5042161
to
a6892d4
Compare
Sorry for misunderstanding, @tscrim. I think it is deprecated appropriately now. |
Documentation preview for this PR (built with commit a6892d4; changes) is ready! 馃帀 |
I think this is ready to go. Any objections to changing it from "needs work" to "positive review"? (The new deprecation doesn't have a corresponding doctest, but since the parameter wasn't used in the first place, I don't think we need to add a doctest.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jhpalmieri Yes, I agree that it is fine without a doctest for it. We can set it to a positive review now.
@OP5642 Please also change this from a draft PR to a regular PR too. |
馃摎 Description
The
rename_vertices
parameter is not mentioned anywhere within the body of the methoddisjoint_union
. We remove the parameter.馃摑 Checklist