[SciPy-Dev] pull requests and code review

Travis Oliphant travis@continuum...
Thu Jan 5 19:08:02 CST 2012


Fair enough.   API users willing to review are golden.  I am happy to suppress my impatience to give users time to see changes.

It makes me wonder if one could set up a system that allows downstream users to voluntarily register to receive notifications on changes to functions or classes that affect them.  

Functions that are heavily used would have more time for people to review the changes. 



--
Travis Oliphant
(on a mobile)
512-826-7480


On Jan 5, 2012, at 6:55 PM, josef.pktd@gmail.com wrote:

> On Thu, Jan 5, 2012 at 7:41 PM, Travis Oliphant <travis@continuum.io> wrote:
>> I don't think there should be any time limit on pull requests.     Who is the *we* that needs time to look at them.   I did take time to look at the changes.   The code changes were not extensive (except for the very nice tests), and it is a welcome change.
> 
> We is whoever is interested and might be affected by the changes. I'm
> looking at all stats pull request and most or all pull requests that
> will have a direct impact on statsmodels.
> 
> It takes some time until I see the notification that a pull request
> has been opened and some time to see whether it might be a change that
> affects statsmodels.
> 
> As soon as a pull request is closed github doesn't send out any
> notifications anymore. So the discussion is closed, except for
> developers that are already participating in the pull request. At
> least that is my impression of github from the past experience.
> 
> 
>> 
>> Your feedback on the use of inspect is very good.   We can take a look at whether or not it method calls were considered and fix it if it does.
> 
> relying on inspect is very fragile, and it depends a lot on the
> details of the implementation, so I'm always sceptical when it's used.
> In this case it compares the hessian signature with the signature of
> the main function, if they agree then everything is fine. But I'm not
> sure it really works in all our (statsmodels) use cases.
> 
>> 
>> If you are interested in continuing to review all the optimize changes, I will make sure and give you time to review in the future.    This is where having a list of interested and available parties for different modules would make a great deal of sense.
> 
> Since statsmodesl is a very heavy user of scipy.optimize, I'm keeping
> almost complete track of any changes.
> 
> Josef
> 
>> 
>> Thanks,
>> 
>> -Travis
>> 
>> 
>> On Jan 5, 2012, at 6:19 PM, josef.pktd@gmail.com wrote:
>> 
>>> Can we keep pull requests open for more than 3 hours, so we actually
>>> have time to look at them.
>>> 
>>> looking at
>>> https://github.com/scipy/scipy/commit/c9c2d66701583984589c88c08c478e2fc6d9f4ec#L1R1213
>>> 
>>> my first guess is that the use of inspect.getargspec breaks when the
>>> hessian is a method attached to a class, as we have almost all our
>>> code in statsmodels.
>>> We just fixed a similar case in scipy.
>>> 
>>> There should be at least time to check whether this kind of suspicions
>>> are justified or not.
>>> 
>>> Josef
>>> _______________________________________________
>>> SciPy-Dev mailing list
>>> SciPy-Dev@scipy.org
>>> http://mail.scipy.org/mailman/listinfo/scipy-dev
>> 
>> _______________________________________________
>> SciPy-Dev mailing list
>> SciPy-Dev@scipy.org
>> http://mail.scipy.org/mailman/listinfo/scipy-dev
> _______________________________________________
> SciPy-Dev mailing list
> SciPy-Dev@scipy.org
> http://mail.scipy.org/mailman/listinfo/scipy-dev


More information about the SciPy-Dev mailing list