Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changes to function reference #383

Closed
wants to merge 1 commit into from
Closed

Changes to function reference #383

wants to merge 1 commit into from

Conversation

peterdesmet
Copy link
Collaborator

This is a suggested update to the function reference:

  • Remove section "Manipulate radar scans" (currently a bit of a mixed bag) and list:
    • integrate_to_ppi() under "Integrate vertical profiles", as it expresses vertically integrated reflectivity (VIR) and requires a vp file (imo less suitable to list under the ppi section then)
    • calculate_param() under "Inspect scans and parameters", right under get_param()
    • apply_mistnet() under "Read radar data" after nexrad_to_odim(), as it is a pvol manipulation
  • Just as scan and param are treated together, also treat vp and vpts together, in 2 sections:
    • "Read vertical profile data", with e.g. read_vpfiles() and read_vpts()
    • "Inspect vertical profiles"
  • Move get_quantity() to "Access profile metadata" (the description mentions "data" as well)
  • Use active verbs "Read", not "Reading"
  • No functions are repeated
  • List example (e.g. example_scan) after other functions in that section

@adokter @bart1 @CeciliaNilsson709 suggestions?

Rendering:

Function_reference_•_bioRad

@peterdesmet peterdesmet requested a review from adokter May 5, 2020 15:58
@adokter
Copy link
Owner

adokter commented May 5, 2020

This is a suggested update to the function reference:

* Remove section "Manipulate radar scans" (currently a bit of a mixed bag) and list:

I liked this section honestly, it are those functions that generate new spatial quantities (either as ppi or scan). And Mistnet acts on scans just as much as on polar volumes. I expect more manipulating functions to be added in the future, so good to have this section.

  * `integrate_to_ppi()` under "Integrate vertical profiles", as it expresses vertically integrated reflectivity (VIR) and requires a vp file (imo less suitable to list under the ppi section then)

The core functionality of this function is much more towards generating a PPI than that it is integrating vertical profiles. It could be under "Project scans and parameters", but I prefer the "manipulate radars scans" because new quantities are being generated.

  * `calculate_param()` under "Inspect scans and parameters", right under `get_param()`

My vote is against this, as the function generates new scan parameters, not inspecting/plotting existing data

  * `apply_mistnet()` under "Read radar data" after `nexrad_to_odim()`, as it is a pvol manipulation

Also voting against this one, the function is segmenting out rain from biology, "reading radar data" doesn't capture that.

* Just as `scan` and `param` are treated together, also treat `vp` and `vpts` together, in 2 sections:
  
  * "Read vertical profile data", with e.g. `read_vpfiles()` and `read_vpts()`
  * "Inspect vertical profiles"

Ok, but in that case rename "Inspect vertical profiles" to "inspect and manipulate vertical profiles", because regularize_vpts(), filter_vpts() and bind_vpts() have little to do with inspection.

* Move `get_quantity()` to "Access profile metadata" (the description mentions "data" as well)

I think we should keep a strict distinction between actual data and metadata. get_quantity() accesses data while all the other functions are higher level thresholding attributes. Suggest moving it to "inspect and manipulate vertical profiles"

* Use active verbs "Read", not "Reading"

ok

* No functions are repeated

ok

* List example (e.g. `example_scan`) after other functions in that section

ok

@bart1
Copy link
Collaborator

bart1 commented May 6, 2020

Hi, I must say I'm not a daily user of this bit of documentation so I'm not sure how valuable it is but I tend to agree with @adokter that some topic on manipulating makes sense as these might be operations people are looking for. It might make sense to also include polar volumes in the title.

I also noticed the topic project scans and parameters now contains a lot that is more about visualizing then projecting (eg, download_basemap, plot, [). For me projecting is quite a specific operation thus maybe this heading should be more generic.

Now the headings seem to be mixture of both the type of object and the type of operation. Would a two level grouping make more sense? Or reporting functions in multiple places where it makes sense (e.g functions that convert from one type to an other)?

@adokter adokter added the sprint label May 27, 2021
@peterdesmet peterdesmet changed the base branch from develop to master May 27, 2021 15:55
@peterdesmet peterdesmet added this to the 0.6.0 milestone May 27, 2021
@peterdesmet peterdesmet closed this Jun 9, 2021
@peterdesmet peterdesmet deleted the function_ref branch June 9, 2021 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants