MAINT: rename MaximumFlowResult.residual to flow #16004
Conversation
fuglede
force-pushed
the
deprecate-residual
branch
from
6473487
to
086a6be
Compare
The attribute `MaximumFlowResult.residual` was poorly named: It in fact represents the flow function of the maximum flow, whereas the residual is given by the difference between the input graph and this flow function. As such, we simply rename it from `residual` to `flow` to avoid the risk of confusion. A different possible fix would be to change the attribute to actually be the residual graph, but that would change its value and therefore lead to a slightly trickier deprecation. The flow is also useful in itself, so for now, we simply include that, and let it be up to the user to make the more or less trivial calculation of the residual if that's ever needed. This closes scipygh-14731
fuglede
force-pushed
the
deprecate-residual
branch
from
086a6be
to
04fbe9b
Compare
[skip azp] [skip circle]
tupui
added
deprecated
|
Great, thanks both for checking. |
|
@h-vetinari a future deprecation to be added to #15765 |
* MAINT: rename MaximumFlowResult.residual to flow The attribute `MaximumFlowResult.residual` was poorly named: It in fact represents the flow function of the maximum flow, whereas the residual is given by the difference between the input graph and this flow function. As such, we simply rename it from `residual` to `flow` to avoid the risk of confusion. A different possible fix would be to change the attribute to actually be the residual graph, but that would change its value and therefore lead to a slightly trickier deprecation. The flow is also useful in itself, so for now, we simply include that, and let it be up to the user to make the more or less trivial calculation of the residual if that's ever needed. This closes scipygh-14731 * MAINT: add stacklevel to deprecation warning [skip azp] [skip circle] Co-authored-by: Pamphile Roy <roy.pamphile@gmail.com>
|
@h-vetinari Note that our policy is flexible. In this case I did not think it was necessary. Feel free to send a quick fix otherwise. |
|
I know that the policy if flexible, and I get that the naming was suboptimal, but just removing the old way of doing things will result in a hard break for everyone who had warning-less code in 1.8. It's not uncommon that people don't upgrade for a while and then bump more than one version in one go. Of course we cannot accommodate that indefinitely, but two releases (~= 1 year) seems like a decent trade-off. I'd be less outspoken if this was just a necessary behaviour-change, but for removing something entirely, we should leave more time IMO. I'd ask @fuglede to do this, since it was their PR and I just didn't see it in time before it got merged. |
|
@h-vetinari First, a maintainer approved (an another member of the triage team), and for such small thing it's more important to have a common voice than debating and have such conversation. We can meet offline to talk about these of course. Second, it took me the same time to make a PR than it certainly took you to comment. So please, next time just do the change. If you really look at it, we have very limited people contributing to SciPy. So let's help everyone by not asking to make extra PRs to fix a single char. |
|
I made the PR and updated the version. Thanks again everyone. All closed here 😃 |
* MAINT: rename MaximumFlowResult.residual to flow The attribute `MaximumFlowResult.residual` was poorly named: It in fact represents the flow function of the maximum flow, whereas the residual is given by the difference between the input graph and this flow function. As such, we simply rename it from `residual` to `flow` to avoid the risk of confusion. A different possible fix would be to change the attribute to actually be the residual graph, but that would change its value and therefore lead to a slightly trickier deprecation. The flow is also useful in itself, so for now, we simply include that, and let it be up to the user to make the more or less trivial calculation of the residual if that's ever needed. This closes scipygh-14731 * MAINT: add stacklevel to deprecation warning [skip azp] [skip circle] Co-authored-by: Pamphile Roy <roy.pamphile@gmail.com>
The attribute
MaximumFlowResult.residualwas poorly named: It in factrepresents the flow function of the maximum flow, whereas the residual
is given by the difference between the input graph and this flow
function. As such, we simply rename it from
residualtoflowtoavoid the risk of confusion.
A different possible fix would be to change the attribute to actually be
the residual graph, but that would change its value and therefore lead
to a slightly trickier deprecation. The flow is also useful in itself,
so for now, we simply include that, and let it be up to the user to make
the more or less trivial calculation of the residual if that's ever
needed.
This closes #14731
I think this is the first time I add a
DeprecationWarning, so I'd appreciateif the prospective reviewer could take a look at the form of that.
cc @czgdp1807 @pmla