Sc greens functions - Githubissues

martinvandriel commented 9 years ago

This pull requests adds a route to request Greens Functions in the decomposition of Minson & Dreger (2008).

There might be some room to avoid code duplication, but I am unsure how best to do it. Essentially there are a few functions in GreensHandler and SeismogramHandler which are the same or very similar (and some overlap with seismograms_raw as well):

parse arguments: checking for duplicates and required arguments as well as default values is the same in the three routes
parse_time_settings is the same in /greens and /seismogram
set_headers ist the same in /greens and /seismogram, besides the default label
get_ttime is the same in /greens and /seismogram
validate geometry is the same in /greens and /seismogram, where it could be simplified in /greens, because greens only works with reciprocal DBs. On the other hand, we can use the same function for both without problem.

martinvandriel commented 9 years ago

So, I merged the duplicated functionality in base class. The only thing I dislike is, that the RawSeismogramHandler now has a bunch of functionality it doesn't need. So maybe this should be split into to generations of classes.

martinvandriel commented 9 years ago

It seems only miniseed is working and saczip gets stuck somewhere.

martinvandriel commented 9 years ago

I can't reproduce these failing tests for 2.7 on my local machine, could this be a problem with the travis setup?

krischer commented 9 years ago

I can't reproduce these failing tests for 2.7 on my local machine, could this be a problem with the travis setup?

I'm surprised it worked on Python 3.x and your local machine - but you already figured the issue out so its all good now.

krischer commented 9 years ago

I renamed the function name, route name and parameters a tiny bit. If that is ok with you, feel free to merge.

Test coverage of the /greens_function route is now pretty high and the I moved a couple more functions to the parent time series routing class.

While I was at it I also made the /raw route async...not clean to do it in this PR but if its just the two of us I guess its ok.

martinvandriel commented 9 years ago

Ok. To me it appears we are good to merge.

krischer / instaseis

Sc greens functions #13