[SciPy-Dev] Hankel transforms, again

[Ralf Gommers]

On Tue, Mar 11, 2014 at 12:19 AM, Tom Grydeland <tom.grydeland at gmail.com>wrote:

>
> On 2014-02-14, at 13:15, Robert Kern <robert.kern at gmail.com> wrote:
>
> > On Fri, Feb 14, 2014 at 9:45 AM, Tom Grydeland <tom.grydeland at gmail.com>
> wrote:
> >> Hi developers,
> >>
> >> This is a repost of a message from December 2008 which gave no useful
> answers.  Since then, I've had 4-5 requests for the code from people who
> had a need for it.  It's not a massive demand, but enough that perhaps
> you'll consider my offer again.
> >>
> >> Since the previous posting, I've also included alternative filters
> thanks to Fan-Nian Kong that are shorter and more accurate when the
> function makes significant changes in more limited intervals. I'm not
> including the code (since it is mostly thousands of lines of tables), but I
> will provide the files to anyone who's interested.
> >
> > Yes, I think we'd be interested. Please do make a PR.
>
> Sorry this has taken a while, I got bogged down with some other stuff.
>
> The changes are, I believe, here:
>
> https://github.com/togry/scipy/compare/signal-hankel-transform
>
> (I'm completely unfamiliar with Git, so bear with me if this should be
> done differently)
>

Hi Tom. The commit looks fine. I guggest you send this as a pull request,
so it's easier to review. A few comments already:
- the API looks slightly awkward, instantiating the class is basically a
do-nothing operation. You'd normally do this with a plain function that has
a ``method='anderson'`` keyword.
- the hankel0, hankel1 and hankel01 methods look unnecessary.
- The file names of all the Python files you add in scipy/signal/ should
start with an underscore, so it's clear that they are private.
- The docstrings could use an example and should be formatted according to
https://github.com/numpy/numpy/blob/master/doc/HOWTO_DOCUMENT.rst.txt
- the ``try: iter(B)`` blocks can be written simply as ``B =
np.asarray(B)``. The way it's now, B as a list will raise an error later on.
- The for-loop over all values of B looks quite inefficient.

Cheers,
Ralf



>
> > Before you do,
> > please double-check the licensing on the new code that you have added.
> > It does look like Anderson's original code is in the public domain
> > (the paper being published as part of Anderson's work at the USGS as a
> > federal employee), so that part is in the clear. Just so we are clear,
> > the lack of copyright statements (work by US federal employees aside)
> > usually means that you have *no license* to redistribute the work, not
> > that there are no restrictions on redistribution.
>
> I couldn't get a clearer statement from Fan-Nian Kong, so I've only
> included the Anderson filters.  There's a reference to Kong's paper in the
> docstrings, however, so adding the filters from whatever sources should be
> simple.
>
> > Thanks!
>
> I hope others find this useful also
>
> > Robert Kern
>
>
> --Tom Grydeland
> _______________________________________________
> SciPy-Dev mailing list
> SciPy-Dev at scipy.org
> http://mail.scipy.org/mailman/listinfo/scipy-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/scipy-dev/attachments/20140317/fe42f745/attachment.html>