rf
rf copied to clipboard
trichter
Add harmonic decomposition to rf
I've added functions for calculating harmonics and for plotting them. So far I haven't made any documentation updates - if this is something you're ok with adding and it looks like it's in decent shape, I'll work on docs and maybe a notebook example.
There's also a commit or two from a small plotting change I made a while back to allow plotting a stacked trace on its own with imaging.plot_rf(); apologies for mixing that in here, I just realized that it ended up on this branch.
Hi, Thanks for contributing more code! Of course, I am OK with adding more of your code. I did not work on harmonic decomposition on my own, though.
Yes, nice of you to add some docs. A tutorial is a big plus. And we will need a test (which could be similar code as in the tutorial or you can reuse other existing code in a stripped down version).
I just pushed some commits to get the tests working again. This branch needs a rebase on master.
I will add some inline comments and will do a more thorough review later.
Thanks, Tom! I did a rebase and addressed those first few comments you gave. I'll work on docs/tests and an example soon.
Ok, I added some documentation (including a tutorial notebook with simple synthetics) and tests - it seems to have broken something in sphinx but I'm not sure what. Let me know what you think.
The broken documentation is a feature of the master branch. I will solve it there.
The documentation build is working again. Also, I was curious why the tests were not running on your branch, I hope this is fixed now. Can you please rebase onto the master branch again?
Thanks, Tom. I made a bit of a mess with the commit history but it looks like the docs are ok now.
I think the rest of the tests are going to keep failing here because of the mtspec requirements which I hopefully fixed in a separate PR? Not sure why they pass on master since conda is definitely not installing mtspec for 3.10 and 3.11.
I fixed the commit history and made some style changes. Looks good to me! Nice documentation too. Thanks! The PR is ready to be merged from my side, if you don't want to add anything.