Renaming of functions - Githubissues

GeoscienceAustralia / ga_sar_workflow

InSAR processing workflow used by Geoscience Australia

Apache License 2.0

3 stars 0 forks source link

Renaming of functions #128

Open adeane-ga opened 3 years ago

adeane-ga commented 3 years ago

In process_ifg.py there are defined functions where we have the opportunity to rename them so that it makes more sense in the context of the script. See below code that shows the run_workflow() function which contains all the processing steps in this script.

Proposition/discussion:

Change calc_int() --> calc_ifg()
Have consistent function prefix in this script. So instead of calc_ generate_ and do_ as listed here: calc_int() generate_init_flattened_ifg() generate_final_flattened_ifg() calc_filt() calc_unw() calc_unw_thinning() do_geocode()

We could choose a consistent name (e.g. calc_ or generate_), what would people prefer?
For the geocode step, we could also add a suffix: e.g. calc_geocoded_ifg() or generate_geocoded_ifg()

https://github.com/GeoscienceAustralia/gamma_insar/blob/329e686def2e996b31e7a5909aee82cd93820b87/insar/process_ifg.py#L66-L84

truth-quark commented 3 years ago

Ooops, that's the result of an on-the-fly schweinfraß naming scheme :-)

How about using create_ as a prefix, as in create_ifg() and create_initial_flattened_ifg()? It might be a bit ugly with geocode, unless changed to something like create_geocoded_thingy()?

Also, what are the team's thoughts on expanding names for code clarity, like changing filt to filter[ed|ing]?

truth-quark commented 3 years ago

Adding another related task: rename error messages to reference new function names, e.g. here: https://github.com/GeoscienceAustralia/gamma_insar/blob/pygamma_workflow/insar/process_ifg.py#L494

adeane-ga commented 3 years ago

Ooops, that's the result of an on-the-fly schweinfraß naming scheme :-)

How about using create_ as a prefix, as in create_ifg() and create_initial_flattened_ifg()? It might be a bit ugly with geocode, unless changed to something like create_geocoded_thingy()?

Also, what are the team's thoughts on expanding names for code clarity, like changing filt to filter[ed|ing]?

Regarding the prefix options, I have run a small poll with the InSAR team and it was a close vote for calc_. for the calc_geocode, it could be changed to calc_geocoded_ifg as you have suggested.
Regarding the filt or filtered, could be a good change to make but haven't discussed it with team at this stage. So could keep it as filt for now.

Generally, these potential name changes might come up a bit, so I guess we could keep this issue open, and cover it all in one go at some stage . Perhaps if they stack up, we can find a nice way to tabulate any function in question.