jakevdp / lombscargle

Efficient, astropy-compatible implementation of the Lomb-Scargle periodogram
BSD 3-Clause "New" or "Revised" License
5 stars 1 forks source link

Bring this into astropy? #1

Open jakevdp opened 8 years ago

jakevdp commented 8 years ago

cc: @eteq @astrofrog

This package is in pretty good shape now I think, though I could add some examples to the docs & unit test some corner-cases. In the meantime, I'd appreciate some fresh eyes on this if you have a chance – let me know if you have feedback on API or things that could be clarified in the docs.

We chatted at PyAstro about the possibility of merging the Lomb-Scargle functionality into astropy. If you still think that makes sense, let me know and I'm happy to start a PR. Otherwise I'm fine keeping this as an affiliated package.

eteq commented 8 years ago

I think putting it into the Astropy core makes sense, as it's a fairly "well-known" or otherwise "standard" algorithm at this point.

One question is where. Maybe stats is a good place? @crawfordsm might have some thoughts there... But it's not exactly clear to me that this is really "statistics" per se, so perhaps that's not a best fit.

Another option is time, as that's is about time series. @taldcroft might have opinions there.

Another option is to put it in photutils. That's an "astropy-managed" (i.e. it started at the beginning and has core devs closely involved) affiliated package so not quite the same as the core, but it might be easier to find that way?

(Will also try to have a closer look at the API, but probably not until next week, post-HST deadline...)

crawfordsm commented 8 years ago

I think the stats package would be an appropriate place for it. I know people are often looking for this functionality and I think it would fit into the other contributions @jakevdp has made there.

taldcroft commented 8 years ago

I was also thinking stats is the best analog.

jakevdp commented 8 years ago

For convenience, I've put a documentation build here: http://jakevdp.github.io/lombscargle/

I'll try to keep it up-to-date.

jakevdp commented 8 years ago

I think this is in pretty good shape now! Shall I open a PR in astropy.stats?

taldcroft commented 8 years ago

@jakevdp - this looks great. It looks like there is agreement for astropy.stats, so please go ahead with the PR there.

jakevdp commented 8 years ago

PR opened here: https://github.com/astropy/astropy/pull/4811