Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible merge with xarray-dsp and other projects #4

Closed
smartass101 opened this issue Aug 9, 2018 · 21 comments
Closed

Possible merge with xarray-dsp and other projects #4

smartass101 opened this issue Aug 9, 2018 · 21 comments

Comments

@smartass101
Copy link
Contributor

As we have discussed in the mailing group, it might be reasonable to merge this project with my smaller project smartass101/xarray-dsp.

The xarray_dsp package in the project currently features mostly functions for frequency and other filters and spectral analysis and a few other wrappers. It mostly targets time-domain analysis.

There are also a few wrappers for wavelets from pycwt, but I guess those wouldn't really fit into xr-scipy since pycwt is not included in scipy. It's also a question whether you want to restrict this project to scipy functions.

I also found kaipak/xrsigproc which also uses some smoothing functions from scipy.ndimage and scipy.convolve for (probably) spatial filtering.
There is also serazing/xscale which also does some signal processing and FFT using mostly scipy again.
Maybe we could contact these projects as well.

@fujiisoup
Copy link
Collaborator

Thanks for contacting me.

Also, I know crusaderky/xarray_extras.
There must be much more small packages that have been developed individually.
For a limited list, see FAQ page of xarray's document page

It is a good idea to merge these packages into one, which would reduce the maintenance and development cost.
Howerver, it is also possible that developers would be flustrated when there is any disagreement in the development direction.

So, I think it would be nice to decide a project goal before contacting to other project managers.
My idea is

  • Only implement functions that are in numpy or scipy.
  • Try to keep API and package structure as similar to those of numpy and scipy as possible.

But honestly, current development and maintenance status are far from satisfactory yet, because of lack of resources.
For example, interpolate module should be deprecated as xarray now natively supports it.
fft module should be updated to follow the recent xarray update (#3).

I am really welcome your possible contribution to this project.
I think xarray_dsp could be implemented into signal module as similar to scipy.signal module.

Any thought is welcome, for the implementation or the project goal.

@smartass101
Copy link
Contributor Author

I'm not sure how closely to follow the scipy/numpy API, because their functions usually expect separate arrays wheres when using xarray many new "higher-level" concepts become easily accessible.
For instance, xarray-dsp mostly follows the scip.signal API for the spectral analysis tools, but is a bit different for the filter routines, because it becomes easy to automatically construct and apply a filter based on the information in the time axis of an array. This is a more of a "high-level" approach than to design filter coefficients separately.

Xscale also seems to focus on a higher-level API for windowing.

Ultimately, I'd focus mostly on what functions are commonly used in actual real-world analysis of data.

@fujiisoup
Copy link
Collaborator

Ultimately, I'd focus mostly on what functions are commonly used in actual real-world analysis of data.

I looked at your filter module, and find high level functions such as highpass are really neat.
On the other hand, an advantage of the similar API to scipy or numpy is that we can greatly lsimplify the document; users do not need to read our document in its details if they know how to use scipy's counterpart. It would be also important and we may need to balance these two.

Can we prepare high level functions as well as scipy's counterpart?

@OriolAbril
Copy link

Hi!

I have created a library that has wrappers for scipy.stats (among other things): https://xarray-einstats.readthedocs.io/en/latest/.

It currently has:

  • wrappers for many numpy.linalg functions
  • wrappers for scipy.stats distributions plus a few functions
  • wrappers for einops functions
  • a numba.guvectorize-decorated version of numpy.histogram for dataarrays

I have mostly added wrappers for things I personally use, also trying to not overlap with your library nor xarray-extras, so it might not make much sense as a group, but the modules are completely independent between them and could be reorganized into independent packages or merged into existing ones. I don't know what your roadmap is nor if there is interest within the xarray for scipy wrappers or similar things, but it could be interesting to collaborate.

In my case, I might add some very thin wrappers on a few special functions that I use often (i.e. expit, logsumexp), and some more guvectorized things, but the things above are mostly what I'll use and what I am willing to maintain.

Also, feedback very welcome!

@fujiisoup
Copy link
Collaborator

Hi. Thank you for the suggestion.

Honestly speaking, i have nowadays no time to maintain this project🥲
And furthermore, most of the features of this package is now implemented into the xarray.

If anyone is interested in developing this package, I'm happy to give the ownership of this repository.
It should be better for the open source community.

@andersy005
Copy link

Honestly speaking, i have nowadays no time to maintain this project🥲
If anyone is interested in developing this package, I'm happy to give the ownership of this repository.
It should be better for the open source community.

@fujiisoup, would you be up for transferring the project to the xarray-contrib organization? I am thinking that It would help increase the overall visibility of the project and in the process, some folks may chip in to help with the development/maintenance of the project moving forward if need be.

@fujiisoup
Copy link
Collaborator

would you be up for transferring the project to the xarray-contrib organization?

@andersy005, Yes, if it is OK to transferring buggy one

@hippalectryon-0
Copy link
Owner

@andersy005 @fujiisoup Is there any update on this transfer ? Despite the state of this repository and xarray-dsp which both haven't been updated in a very long time, theyr'e still the first results that appear on google when searching for ways to apply scipy functions to xarrays, as many still aren't implemented on the main xarray repository.
Transferring ownership to an active dev/org would allow us to start merging all the MRs and forks :)

@fujiisoup
Copy link
Collaborator

@hippalectryon-0 Thank you for commenting. As I get busier the development has been suspended. I am happy to transfer the ownership to someone who can keep developing. It would be appreciated if you could suggest any possible owner / repository that I can send the ownership.

@hippalectryon-0
Copy link
Owner

I'd be happy to (at least for now) take ownership of the repository. I could then bump the python versions to support 3.8+, merge the MRs, and re-contact xarray-contrib and @andersy005 to see what can be done in the longer term.

@fujiisoup
Copy link
Collaborator

I appreciate that you can take the ownership. I sent an invitation to take the ownership for this repository. I hope the development still continues.

@hippalectryon-0
Copy link
Owner

Thanks ! I haven't received anything so far, but maybe github is just being slow. I'll ping you in a few hours if I still don't see it :)

@hippalectryon-0
Copy link
Owner

@fujiisoup I haven't received anything, can you double -check ? Thanks :)

@fujiisoup
Copy link
Collaborator

Looks like you already have a repository named xr-scipy? I think we need to delete it first to transfer the ownership of this repository

@hippalectryon-0
Copy link
Owner

I've renamed my fork, let me know if that works now.

@fujiisoup
Copy link
Collaborator

Looks like renaming the fork is not sufficient.
https://docs.github.com/en/github-ae@latest/repositories/creating-and-managing-repositories/transferring-a-repository
I think we need to delete the fork.

@hippalectryon-0
Copy link
Owner

done

@fujiisoup
Copy link
Collaborator

Thank you. I think the request is sent to your place.

@hippalectryon-0
Copy link
Owner

Perfect, looks like it worked this time :)

@smartass101
Copy link
Contributor Author

Hi, on my side I don't have much time right now to actively contribute to cleaning up the code, but I'd be happy to at least offer guidance on merging xarray_dsp, xrrandom and other small scipy wrappers I have into this. And possibly I can dig up some doc resources I have. I still actively use them and I think many other people will find them very useful, especially if the scipy wrappers become less fragmented.
Thanks for stepping up @hippalectryon-0 :)

@hippalectryon-0
Copy link
Owner

I'm closing this issue in favor of #17 , #18 , #19 since we had quite a bit of off-topic discussion here :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants