jpjones76 / SeisIO.jl

Julia language support for geophysical time series data
http://seisio.readthedocs.org
Other
47 stars 21 forks source link

writesac() to support user-defined fname? #18

Closed xtyangpsp closed 5 years ago

xtyangpsp commented 5 years ago

I am writing SeisChannel of cross-correlation results to sac. However, the id is not standard NET.STA.LOC.CHAN format. It includes two station information, e.g., TA.V04C..BHZ.TA.V05C..BHZ. The current fill_sac() generates file name based on the ID field and is not working correctly in my case. I think making writesac() to support more generic filenames or filenamebase should be useful.

Thanks, Xiaotao

jpjones76 commented 5 years ago

Hi Xiaotao,

Can you please provide a minimum working example to reproduce the issue with the file ID being generated incorrectly?

I can definitely change writesac to allow user-specified file names. Can you please provide a little more information about how you'd like the file names to work?

xtyangpsp commented 5 years ago

Hi Josh,

I modified your writesac() and fill_sac() to handle CorrData headers, with the option of specifying file names. I wrote a script, as part of the SeisConvert package, to convert CorrData to SeisData. LInk to the code: https://github.com/xtyangpsp/SeisConvert.jl/src/corr2seis.jl . The modified sac writing codes are under: https://github.com/xtyangpsp/SeisConvert.jl/src/unilities.jl. They are re-named to: writesac_rich() and fill_sac_rich() to handle data with rich header information in misc field. Here is the link to a test CorrData jld2 file: https://app.box.com/s/vw8p07z74b1imueb02fd4g159dxo6q2e

The codes are working as part of SeisConvert package. You are welcome to incoorporate them in SeisIO if you think they are helpful.

Xiaotao

jpjones76 commented 5 years ago

Hi Xiaotao,

I can change writesac() and fill_sac() to allow a user-defined file name string, but it's not possible to incorporate code into SeisIO that defines a custom Type from a dependent module as the default case. That would almost certainly lead to a circular dependency.

Instead, I think that what you're doing now is the right approach: namely, importing and overloading write_sac(), as you've done in your SeisConvert.jl package.

In fact, would it be possible to merge SeisConvert and your modified write_sac() routine into @tclements package SeisNoise.jl? Tim's code is excellent and looks better every time I check.

xtyangpsp commented 5 years ago

Hi Josh,

I think you are right that adding support for a custom data type is now ideal, especially for a dependent Type. Adding an option to allow user-defined file name(s) or namebase would be a good ideal, if it’s not too difficult for you.

SeisIO is an excellent package. So does SeisNoise. SeisConvert, though, has its own mission, at least for now. My solution now works for the purpose.

Thanks, Xiaotao On Jul 15, 2019, 18:18 -0400, Joshua Jones notifications@github.com, wrote:

Hi Xiaotao, I can change writesac() and fill_sac() to allow a user-defined file name string, but it's not possible to incorporate code into SeisIO that defines a custom Type from a dependent module as the default case. That would almost certainly lead to a circular dependency. Instead, I think that what you're doing now is the right approach: namely, importing and overloading write_sac(), as you've done in your SeisConvert.jl package. In fact, would it be possible to merge SeisConvert and your modified write_sac() routine into @tclements package SeisNoise.jl? Tim's code is excellent and looks better every time I check. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

jpjones76 commented 5 years ago

Hi Xiaotao,

Sounds good. Within the next few days, I'll push a change to master that allows custom filenames when writing GphysChannel objects to SAC.

jpjones76 commented 5 years ago

This change was pushed live tonight. I'm going to close this issue as there's nothing more that can be done at present.