cbeleites / hyperSpec

R package hyperSpec can now be found at https://github.com/r-hyperspec/hyperSpec
http://r-hyperspec.github.io/hyperSpec/
GNU General Public License v3.0
22 stars 11 forks source link

Rename functions (2): wl_ #300

Closed GegznaV closed 3 years ago

GegznaV commented 3 years ago

This issue complements https://github.com/cbeleites/hyperSpec/issues/208#issuecomment-684085699.

Most of the functions that directly process @wavelength should start with prefix wl_. There are several wavelength-related functions in hyperSpec that, I'm certain, should be renamed (group A):

Status Current name Suggested name Comment
❗ needs discussion all_wl wl_all
❗ needs discussion any_wl wl_any

These will be solved in issue #309:

Status Current name Suggested name Comment
moved to #309 orderwl wl_sort The output is more consistent with base::sort() than with base::order().
moved to #309 wl.eval wl_eval
moved to #309 wlconv wl_convert_units 1. More explicit name gives more clarity. 2. I'd like to make this function S3 generic with method for numeric, integer, and hyerSpec. 3. I'd like to rename the arguments, e.g., pointsobj or x, srcfrom, dstto, laserreference_wl.
moved to #309 guess.wavelength extract_numbers Function is applicable not only to extract wavelengths but also numbers in general. So the new name explains the essence of the function better. Equivalent function: readr::parse_number().

These wavelength-related functions should not be renamed (group B):

a. wl b. nwl c. wl2i d. i2wl

I feel ambivalent about renaming these functions (group C), but to be more consistent, they should also start with wl_ (it would be easier to find these functions by using auto-completion). On the other hand, they are wrapped with wlconv()/wl_convert_units().

  1. ev2freq
  2. ev2invcm
  3. ev2nm
  4. ev2raman
  5. freq2ev
  6. freq2invcm
  7. freq2nm
  8. freq2raman
  9. invcm2ev
  10. invcm2freq
  11. invcm2nm
  12. invcm2raman
  13. nm2ev
  14. nm2freq
  15. nm2invcm
  16. nm2raman
  17. raman2ev
  18. raman2freq
  19. raman2invcm
  20. raman2nm
bryanhanson commented 3 years ago

The proposed changes to group A & B make perfect sense to me. Group C: are these functions exported? I don't think I have run into them.

cbeleites commented 3 years ago
GegznaV commented 3 years ago

@bryanhanson yes, they are exported

GegznaV commented 3 years ago

At least for now I'll leave the current names of all_wl and any_wl but at least temporarily deprecate unit conversion functions.

GegznaV commented 3 years ago

This issue is solved.

We leave all_wl() and any_wl() as they work on logical matrix-like object and not on @wavelength slot of a hyperSpec object.