csp icon indicating copy to clipboard operation
csp copied to clipboard

First cut of seismic waveform example

Open ptomecek opened this issue 1 year ago • 2 comments

It's in a reasonably presentable state, but could always use further improvements.

ptomecek avatar May 22 '24 18:05 ptomecek

@robambalu Latest commit should incorporate all your feedback

ptomecek avatar May 23 '24 18:05 ptomecek

@AdamGlustein All your other feedback is incorporated, but I would also love an answer to the units question, so leaving that one open for now.

ptomecek avatar May 23 '24 20:05 ptomecek

EasySeedLinkAdapter is pretty useless on its own, i would combine with EasySeedLinkAdapterImpl as here.

Also EasySeedLinkAdapterImpl / _EasySeedLinkAdapterImpl , EasySeedLinkAdapter / _EasySeedLinkAdapter is pretty awful naming unless the goal is to confuse the reader 😆

I will have a look at this - I probably copied the wrong example

ptomecek avatar May 30 '24 13:05 ptomecek

@timkpaine I've updated the adapter definition - agree it's much more straightforward now. @AdamGlustein I've added a link to the USGS FAQ on seismography that has more background information about the space for lay people.

ptomecek avatar May 31 '24 12:05 ptomecek

  • [ ] Needs tests
  • [ ] README.md in 07_end_to_end similar to this
  • [ ] Outputs stripped (meh) or notebook stored in LFS (also meh)

As discussed offline, always keen on testing, let's discuss strategies next week. I will also follow up on the longer term question about binary assets in the repo and possible move to LFS or separate repo for binary-heavy examples/notebooks.

ptomecek avatar May 31 '24 13:05 ptomecek

  • [ ] Needs tests
  • [ ] README.md in 07_end_to_end similar to this
  • [ ] Outputs stripped (meh) or notebook stored in LFS (also meh)

As discussed, we will work on tests separately. I think we can also put together a README for the section in a separate PR.

ptomecek avatar Jun 06 '24 14:06 ptomecek