[Numpy-discussion] what to clean up, what to leave as is

2020-01-24 Thread Ralf Gommers
Hi all,

It's great to see that people are jumping at the chance to clean up Python
2 support. I would however caution about overdoing it on other cleanups. As
a reminder, we normally do not want pure style PRs (e.g. PEP8 cleanups),
because they make the code history (git blame, commits on particular files,
etc.) harder to work with, have review overhead, and may introduce new bugs
for little gain.

Imho that same rationale applies to things like converting strings to
f-strings. There's of course some gray area, for example removing "from ...
import *" can guard against accidentally exposing new API, so can be
considered a valuable cleanup.

As a separate/additional point: numpy.distutils and numpy.f2py are largely
untested, PRs are hard to test locally because of platform-specific code,
and changes often introduce regressions. So even for some cleanups that are
okay for other files, please do not do them on those modules.

Cheers,
Ralf
___
NumPy-Discussion mailing list
NumPy-Discussion@python.org
https://mail.python.org/mailman/listinfo/numpy-discussion


Re: [Numpy-discussion] what to clean up, what to leave as is

2020-01-24 Thread Charles R Harris
On Fri, Jan 24, 2020 at 9:46 AM Ralf Gommers  wrote:

> Hi all,
>
> It's great to see that people are jumping at the chance to clean up Python
> 2 support. I would however caution about overdoing it on other cleanups. As
> a reminder, we normally do not want pure style PRs (e.g. PEP8 cleanups),
> because they make the code history (git blame, commits on particular files,
> etc.) harder to work with, have review overhead, and may introduce new bugs
> for little gain.
>
> Imho that same rationale applies to things like converting strings to
> f-strings. There's of course some gray area, for example removing "from ...
> import *" can guard against accidentally exposing new API, so can be
> considered a valuable cleanup.
>
> As a separate/additional point: numpy.distutils and numpy.f2py are largely
> untested, PRs are hard to test locally because of platform-specific code,
> and changes often introduce regressions. So even for some cleanups that are
> okay for other files, please do not do them on those modules.
>
>
I do like f-strings, they can make the code simpler and more readable.

Chuck
___
NumPy-Discussion mailing list
NumPy-Discussion@python.org
https://mail.python.org/mailman/listinfo/numpy-discussion


Re: [Numpy-discussion] what to clean up, what to leave as is

2020-01-24 Thread Kevin Sheppard
I think some types of clean-ups, for example, imports, are pretty low cost,
low risk and don't have much bearing on and it might be best to do them all
at once.

f-strings are also pretty simple but can be abused to the detriment of code
around (for example, moving a string defined in a variable outside a
function into the function just so it can have an f-string).

I think more general code clean-up, for example, removing list around
iterators (e.g., list(map(f, i)) should be mostly avoided unless there is a
compelling case to prefer the iterator for performance reasons.

Kevin


On Fri, Jan 24, 2020 at 5:29 PM Charles R Harris 
wrote:

>
>
> On Fri, Jan 24, 2020 at 9:46 AM Ralf Gommers 
> wrote:
>
>> Hi all,
>>
>> It's great to see that people are jumping at the chance to clean up
>> Python 2 support. I would however caution about overdoing it on other
>> cleanups. As a reminder, we normally do not want pure style PRs (e.g. PEP8
>> cleanups), because they make the code history (git blame, commits on
>> particular files, etc.) harder to work with, have review overhead, and may
>> introduce new bugs for little gain.
>>
>> Imho that same rationale applies to things like converting strings to
>> f-strings. There's of course some gray area, for example removing "from ...
>> import *" can guard against accidentally exposing new API, so can be
>> considered a valuable cleanup.
>>
>> As a separate/additional point: numpy.distutils and numpy.f2py are
>> largely untested, PRs are hard to test locally because of platform-specific
>> code, and changes often introduce regressions. So even for some cleanups
>> that are okay for other files, please do not do them on those modules.
>>
>>
> I do like f-strings, they can make the code simpler and more readable.
>
> Chuck
> ___
> NumPy-Discussion mailing list
> NumPy-Discussion@python.org
> https://mail.python.org/mailman/listinfo/numpy-discussion
>
___
NumPy-Discussion mailing list
NumPy-Discussion@python.org
https://mail.python.org/mailman/listinfo/numpy-discussion


Re: [Numpy-discussion] what to clean up, what to leave as is

2020-01-24 Thread Charles R Harris
On Fri, Jan 24, 2020 at 10:42 AM Kevin Sheppard 
wrote:

> I think some types of clean-ups, for example, imports, are pretty low
> cost, low risk and don't have much bearing on and it might be best to do
> them all at once.
>
> f-strings are also pretty simple but can be abused to the detriment of
> code around (for example, moving a string defined in a variable outside a
> function into the function just so it can have an f-string).
>
> I think more general code clean-up, for example, removing list around
> iterators (e.g., list(map(f, i)) should be mostly avoided unless there is a
> compelling case to prefer the iterator for performance reasons.
>
>
I tend to see that as a matter of timing. At some point such things become
archaic and unfamiliar as developers who grew up with modern Python become
more common. But there is no rush.

The problem with  code history is trickier, and almost unavoidable over
time. The information is still there, but it is harder to dig it out.
Probably there need to be better tools to deal with that. This is
especially apparent from the early C code cleanup that I did just to get
more modern and readable code styling. My name is all over code that I
didn't write, just formatted. But the cleanup was necessary for maintenance
if nothing else.



Chuck
___
NumPy-Discussion mailing list
NumPy-Discussion@python.org
https://mail.python.org/mailman/listinfo/numpy-discussion


Re: [Numpy-discussion] what to clean up, what to leave as is

2020-01-24 Thread Sebastian Berg
On Fri, 2020-01-24 at 10:57 -0700, Charles R Harris wrote:
> 
> 
> On Fri, Jan 24, 2020 at 10:42 AM Kevin Sheppard <
> kevin.k.shepp...@gmail.com> wrote:
> > I think some types of clean-ups, for example, imports, are pretty
> > low cost, low risk and don't have much bearing on and it might be
> > best to do them all at once. 
> > 
> > f-strings are also pretty simple but can be abused to the detriment
> > of code around (for example, moving a string defined in a variable
> > outside a function into the function just so it can have an f-
> > string).
> > 
> > I think more general code clean-up, for example, removing list
> > around iterators (e.g., list(map(f, i)) should be mostly avoided
> > unless there is a compelling case to prefer the iterator for
> > performance reasons. 
> > 
> > 
> 
> I tend to see that as a matter of timing. At some point such things
> become archaic and unfamiliar as developers who grew up with modern
> Python become more common. But there is no rush.
> 

Honestly, I am OK with simply not cleaning up some stuff. It was a
quick thought.
But, if we ever get around doing it, it seems like a good time now when
we already have a big code churn do to Py2 removal.

> The problem with  code history is trickier, and almost unavoidable
> over time. The information is still there, but it is harder to dig it
> out. Probably there need to be better tools to deal with that. This
> is especially apparent from the early C code cleanup that I did just
> to get more modern and readable code styling. My name is all over
> code that I didn't write, just formatted. But the cleanup was
> necessary for maintenance if nothing else.

Code history is a problem and it is annoying that code attribution is
obfuscated by it.
The argument for doing style cleanups is that if we do not use modern
patterns in large chunks of the code, the old patterns are likely to
repeated indefinitely. At least within their files [0].

Tools are maybe getting a bit better (e.g. github allows to click on
"view blame before this, although I doubt we ever get somewhere were
they will figure out that something was "just" a style cleanup.

- Sebastian


[0] An example are pytest vs. nose testing styles, where it is unlikely
we ever lose the complexity of keeping random snippets of nose style
around (which definitely will confuse new contributors).

> 
> 
> Chuck
> ___
> NumPy-Discussion mailing list
> NumPy-Discussion@python.org
> https://mail.python.org/mailman/listinfo/numpy-discussion


signature.asc
Description: This is a digitally signed message part
___
NumPy-Discussion mailing list
NumPy-Discussion@python.org
https://mail.python.org/mailman/listinfo/numpy-discussion


[Numpy-discussion] Deprecate unused but exposed C-API functions

2020-01-24 Thread Sebastian Berg
Hi all,

I would like to deprecate three C-API functions (more may come later)
in https://github.com/numpy/numpy/pull/15427

The functions are:
  * PyArray_GetArrayParamsFromObject
  * PyUfunc_GenericCall
  * PyUFunc_SetUsesArraysAsData

I could not find downstream usage for any of these and they seem useful
only on first glance to me.

The first one is too complex (regarding scalars), simply converting to
an array first should be just as well.
The second one is almost identical to `PyObject_Call`. And the third
one is not even documented and while the idea is sound, I am not sure
that there are any cases where it is useful [0]

My idea is to add the deprecation warning now, and if nobody notices it
remove them as soon as 1.19 is released.

- Sebastian


[0] It seems useful for user datatypes/loop, but using it for those
does not seem possible (or at least straight forward), since they are
usually stored in a way not accessible through this function.


signature.asc
Description: This is a digitally signed message part
___
NumPy-Discussion mailing list
NumPy-Discussion@python.org
https://mail.python.org/mailman/listinfo/numpy-discussion


Re: [Numpy-discussion] Deprecate unused but exposed C-API functions

2020-01-24 Thread Charles R Harris
On Fri, Jan 24, 2020 at 11:29 AM Sebastian Berg 
wrote:

> Hi all,
>
> I would like to deprecate three C-API functions (more may come later)
> in https://github.com/numpy/numpy/pull/15427
>
> The functions are:
>   * PyArray_GetArrayParamsFromObject
>   * PyUfunc_GenericCall
>   * PyUFunc_SetUsesArraysAsData
>
> I could not find downstream usage for any of these and they seem useful
> only on first glance to me.
>
> The first one is too complex (regarding scalars), simply converting to
> an array first should be just as well.
> The second one is almost identical to `PyObject_Call`. And the third
> one is not even documented and while the idea is sound, I am not sure
> that there are any cases where it is useful [0]
>
> My idea is to add the deprecation warning now, and if nobody notices it
> remove them as soon as 1.19 is released.
>
>
I don't think C-API functions should be removed. Deprecation is fine, but
removal changes the customary order of the functions. If they cost nothing
to maintain, just leave them alone with a note/documentation that there are
preferable alternatives.

Chuck
___
NumPy-Discussion mailing list
NumPy-Discussion@python.org
https://mail.python.org/mailman/listinfo/numpy-discussion


Re: [Numpy-discussion] Deprecate unused but exposed C-API functions

2020-01-24 Thread Sebastian Berg
On Fri, 2020-01-24 at 17:56 -0700, Charles R Harris wrote:
> 
> 
> On Fri, Jan 24, 2020 at 11:29 AM Sebastian Berg <
> sebast...@sipsolutions.net> wrote:
> > Hi all,
> > 

> > 
> > My idea is to add the deprecation warning now, and if nobody
> > notices it
> > remove them as soon as 1.19 is released.
> > 
> 
> I don't think C-API functions should be removed. Deprecation is fine,
> but removal changes the customary order of the functions. If they
> cost nothing to maintain, just leave them alone with a
> note/documentation that there are preferable alternatives.
> 

To be clear: removal in this sense for me would mean that they exist as
a stub which always raises an error.
In principle they could be set to NULL as well, but I think the always-
error solution is just as good probably.

- Sebastian


> Chuck
> ___
> NumPy-Discussion mailing list
> NumPy-Discussion@python.org
> https://mail.python.org/mailman/listinfo/numpy-discussion


signature.asc
Description: This is a digitally signed message part
___
NumPy-Discussion mailing list
NumPy-Discussion@python.org
https://mail.python.org/mailman/listinfo/numpy-discussion