[SciPy-Dev] Newbie question: submitting PRs, review

[Ralf Gommers]

On Sun, Feb 16, 2020 at 6:46 PM Tom Roderick <thomas.roderick at gmail.com>
wrote:

> So is the way forward to:
> (1) Write a minimal PR
> (2) Bring open discussion?
>
> The issue appears to be pretty old, and now I'm concerned that people may
> be expecting one metric and getting another!
>

True. The alternatives would be to either add a FutureWarning before making
the switch, or deprecating and then introducing the right metric under a
different name.

I think if it's really a clear cut bug, we can make the switch and add an
entry in the "backwards incompatible changes" section of the release notes
to explain it.


>
> Happy to roll up my sleeves, want to make sure there isn't another blocker
> here outside of a working branch?
>

I think the PR I linked to made the same change you're proposing, but it
was really large and got abandoned. If you make a small PR with just this
change and a clear explanation, I think there won't be another blocker.

Cheers,
Ralf


>
> On Sat, Feb 15, 2020 at 8:00 PM Ralf Gommers <ralf.gommers at gmail.com>
> wrote:
>
>>
>>
>> On Fri, Feb 14, 2020 at 10:52 PM Tom Roderick <thomas.roderick at gmail.com>
>> wrote:
>>
>>> Hi folks
>>>
>>> This is my first time mailing the list, so forgive me if I come across
>>> naive -- it is because I am!
>>>
>>
>> Hi Tom, welcome!
>>
>>
>>> I noticed that the metric in scipy.spatial.distance.sokalmichener is the
>>> wrong metric, as per many sources (e.g.
>>> https://citeseerx.ist.psu.edu/viewdoc/download?doi=10.1.1.352.6123&rep=rep1&type=pdf)
>>> -- the rogerstanimoto dissimilarity is in its place. I'd like to correct
>>> it, but am not 100% sure I understand the process.
>>>
>>
>> In this case there's some previous discussion on this topic. This is the
>> relevant bug report: https://github.com/scipy/scipy/issues/2011. And
>> this identifies more issues and talks about how we should handle them:
>> https://github.com/scipy/scipy/pull/3163#issuecomment-34224341.
>>
>>>
>>> So, wanted to reach out to the mailing list and hopefully get some
>>> mentoring. Is the Scipy process as simple as: fix code, add unit test,
>>> submit Github PR?
>>>
>>
>> It can be, but API changes or backwards incompatible changes usually are
>> more involved (discussion wise mainly). I think you can submit the minimal
>> required fix here though, and add some more references you found to issue
>> 2011. Regarding process there's some more guidance in the contributor
>> guide,
>> http://scipy.github.io/devdocs/dev/contributor/contributor_toc.html, in
>> particular the "development workflow" section.
>>
>> Cheeers,
>> Ralf
>>
>>
>>
>>> Best,
>>> -Tom
>>> _______________________________________________
>>> SciPy-Dev mailing list
>>> SciPy-Dev at python.org
>>> https://mail.python.org/mailman/listinfo/scipy-dev
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/scipy-dev/attachments/20200216/33619e6b/attachment-0001.html>