kasra-hosseini / obspyDMT

A Python toolbox for the query, retrieval, processing and management of seismological data sets, including very large, heterogeneous and/or dynamically growing ones.
http://kasra-hosseini.github.io/obspyDMT/
Other
88 stars 58 forks source link

Sort help options alphabetically #6

Closed john-robert closed 9 years ago

john-robert commented 9 years ago

I like the help menu of obspyDMT but I personally find it annyoing to scroll down and up all the time just because I know the name of the option but not where it was placed. An alphabetical order would ease things and is just 10 minutes of work.

Addtionally:

So the options maybe need some love.

kasra-hosseini commented 9 years ago

Hi John,

Thanks for your message. Some comments below:


it annyoing to scroll down and up all the time just because I know the name of the option but not where it was placed. An alphabetical order would ease things and is just 10 minutes of work.

That is a good idea! and as you said it is just about reordering the options in obspyDMT. Just one tip that whenever I personally want to check for an option I usually use:

./obspyDMT.py --help | grep "what I like to know"

Still, the idea of alphabetic order works better. At this moment, the options are ordered based on the functionality, i.e.: event, station, stationxml and etc are close to each other to know what functionalities are available for each category. The problem with alphabetic order is that we lose this grouping; however, by wisely selecting the name, this can be achieved as well.


including another option to allow users to read-in an already existing event_file

Great! can you send a pull request for that? Would you send an example for that as well? I can include it to the README file


I don't understand the option "--cut_time_phase" at all. Could you explain to me what is intended and, furthermore, what is cut ? Are the given phases (P, Pdiff, PKiKP) just a collection or all available things, how are they related to that option

By default, obspyDMT cuts the time before and after the origin time (or based on the defined time span in continuous requests). This options makes it possible to cut the time before and after the arrival time of different seismic phases. The reason that I chose P and Pdiff is that they are the first arrivals and cover almost all epicentral distances.


I just talked to Fabrice and showed him the tool, he also likes it and has some ideas. However, he thinks a tiny example at every option would make a big difference. E.g. for event_circle do I have to put them in apostrophes or not ...

I tried to add some information for each option. For instance for event_circle that you mentioned:

--event_circle=EVENT_CIRCLE
                        search for all the events within the defined circle,
                        syntax: <lon>/<lat>/<rmin>/<rmax>. May not be used
                        together with rectangular bounding box event
                        restrictions (event_rect).

so syntax will tell you how you should write it, e.g. 0/0/30/110 which are lon/lat/rmin/rmax...but if you found some options that is not well defined just let me know. It is indeed helpful to have some examples for each, and if you already have some, just include and send a pull request.


Thanks for your efforts. It is great that you are looking into the code, I am sure you can enhance/add different functionalities. It would be great if we try to have only one version which has all the functionalities that we include. I know that this is the case so far, just to emphasize. In this way, we will have one clean version that has everything.

All best, KH

john-robert commented 9 years ago

Salut,

Regarding the alphabetical order Yes indeed, I recognised that there is a group-based order but again, if one wants to put the command line together which easily can be up to 10 options and you know what you are looking for, you always end up scrolling the hell up and down (and using grep each time is rather not practical). So I agree, choosing wisely a name would at least garantee a small 'distance' between two related options. I would invest that time and maybe also do some tiny examples more to show you what I/Fabrice mean (because obviously there already descriptions, but I recognise - a trend I see at the AWI and in Saint-Denis - senior scientists more and more switch to python/obspy/obspyDMT, as it does all the jobs just beautifully they were used to do in different programs. Meaning, for them it is not as natural as for you (us) to use python (although this specific help syntax is a rather general one). So to make it more attractive to a hand full of users, it's a good idea to put some love into the options). I will do an extra pull-request for that.

Event read-in possibility Let's do as you said. For this, I would send a different pull request.

--cut_time_phase Okay, I understand. So it takes a specific event, all stations you chose and calculates for each station-event-pair the traveltime of the P/Pdiff-phases. These times are then taken as positiv offsets to cut_before/cut_after. Is that right? So with a hand full of stations (say RHUM-RUM) the retrieved waveforms of the stations - for one event - would show different absolute times ? (Sorry if I annoy you but I need to be sure what is done)

Event output One thing I was a bit 'disappointed' about, the event output in the shell is line by line, it would make better sence - for more clearity (especially for the choosing events option) - to make it as a table. I actually could implement such a thing as I wrote this already for my personal stock. Surprisingly, I didn't asked, Fabrice also would like this feature. So maybe you can consider such a thing and if you'd like it, give me a hint and I will make a pull-request as I obviously will not have enough branches by then. :)

kit John

kasra-hosseini commented 9 years ago

Hi John,


choosing wisely a name would at least garantee a small 'distance' between two related options.

I think that is the way to go. Maybe for the time-being we can proceed as follow:

order the options alphabetically as the options are named at this moment. At some moments, we can make a "deprecated feature" or so, in which we change the options names smartly but we still support the previous options (to be backward compatible). Later one, after 1 or 2 releases, we move on to the new names.


So to make it more attractive to a hand full of users, it's a good idea to put some love into the options

Very much like the idea! It would be great if you make a pull request for that (as you said), and it will hopefully attract senior scientists. Great!


These times are then taken as positiv offsets to cut_before/cut_after. Is that right? So with a hand full of stations (say RHUM-RUM) the retrieved waveforms of the stations - for one event - would show different absolute times ? (Sorry if I annoy you but I need to be sure what is done)

You do not annoy me! really! it is great that you are looking into the options and it always pays off that two people look at a code...and the code will be refined step-by-step. OK, back to your question: Yes, it is like a positive offsets to cut the time before and after the first arrival times. Depends on the source-station geometry, it calculates the arrival times of P or Pdiff and cuts around those. In other words, you see that in all seismograms the first arrival comes at the same time relative to the start time of the trace.


Surprisingly, I didn't asked, Fabrice also would like this feature. So maybe you can consider such a thing and if you'd like it, give me a hint and I will make a pull-request as I obviously will not have enough branches by then.

I did not fully understand, you mean when it shows the event information? in the first step of the code? can you make a quick example for that?

All best, Kasra

john-robert commented 9 years ago

Hi Kasra,

at least today I won't stop giving you some extra work. :)


1. Okay, I understood with the --cut_time_phases now. Nice option. Cool to retrieve waveforms. I was a bit confused because you wrote min_date/max_date are changed .. but I get it now.

2. I made a pull-request for the new option "--read_catalog". Works fine, is cool. Unfortunately, I recognised that this pull-request does also carry changes from my first pull-request, which brings me to my third point.

3. Okay, normally I do like so.

So I guess that's about right. However, establishing different branches does not make 2 or 3 or ... files of e.g. obspyDMT.py. That is, no matter in which branch I am and work on implementations, I still have just one file in total! That is why my changes for the 1st pull-request are present in the 2nd as well (okay, each time I could by hand copy an 'origin' master obspyDMT.py and start from there to make my chances. But this is inconvinient and I believe not intended like so). So how do you do that, are there automatically created multiple files for each branch, for example in "./git" ? What do I miss?

4. For the thing with the ouput. By now, for each event and each value there is one line in the shell. Just as you coded it. What we mean is an actual table as output. Like one header line and then only one line per event! Pretty simple .. I will make that and send you an example.

Guess that's it.

john-robert commented 9 years ago

Okay, never mind that:

3. Okay, normally I do like so.

git checkout master (all commands for my obspyDMT repo)
git branch branchname
git checkout branchname
...

I got it.


I also promise to read more carefully so things don't have to be repeated (anymore).

kasra-hosseini commented 9 years ago

Sorry for the late response but I am happy that everything is fine with github now. I guess the only remaining part (correct me if I am wrong) from your message is:

For the thing with the ouput. By now, for each event and each value there is one line in the shell. Just as you coded it. What we mean is an actual table as output. Like one header line and then only one line per event! Pretty simple .. I will make that and send you an example.

I very much like the idea, I guess it will make the output really nice. Looking forward for your pull request on this ;)

All best, Kasra

kasra-hosseini commented 9 years ago

Hi John,

I am trying to manage the issues. What is the status for this one?

Best, Kasra

john-robert commented 9 years ago

Hi Kasra,

Error: -12345 lazy programmer postponed sorting options. zero work accomplished. process stucked. nothing compiled.

On 12 January 2015 at 17:23, Kasra Hosseini notifications@github.com wrote:

Hi John,

I am trying to manage the issues. What is the status for this one?

Best, Kasra

— Reply to this email directly or view it on GitHub https://github.com/kasra-hosseini/obspyDMT/issues/6#issuecomment-69568536 .

kasra-hosseini commented 9 years ago

:D ok...that is also not the highest priority. Thanks for the update!