wincowgerDEV / OpenSpecy-package

Analyze, Process, Identify, and Share, Raman and (FT)IR Spectra
http://wincowger.com/OpenSpecy-package/
Creative Commons Attribution 4.0 International
27 stars 11 forks source link

Favored dependencies (readr, dplyr, signal, ...) #6

Closed zsteinmetz closed 3 years ago

zsteinmetz commented 3 years ago

@wincowgerDEV, do you happen to know if read_table() is the only function you used from the readr package? If so, I'd suggest to use read.table() instead to limit dependencies a little:

https://github.com/wincowgerDEV/OpenSpecy/blob/f868ab5e79e354a2845a7d887e8ace7573a4d223/R/read_spectra.R#L37-L38

wincowgerDEV commented 3 years ago

Yeah, that is the only use. Good catch. Also, some of the other libraries are only being used for 1 or two things that might be able to be bypassed, smoother, for example, is super annoying because it masks functions in dplyr and we only really want the "filter" function from it which is a simple SG filter that we might as well implement in the OS library with another name.

zsteinmetz commented 3 years ago

Also, some of the other libraries are only being used for 1 or two things that might be able to be bypassed, smoother, for example, is super annoying because it masks functions in dplyr and we only really want the "filter" function from it which is a simple SG filter that we might as well implement in the OS library with another name.

I'll look into that later. We could also use prospectr. Will this affect validation?

wincowgerDEV commented 3 years ago

I'll need to add any changes to the validation script for validating the whole routine but I don't think that is a problem if it helps.

zsteinmetz commented 3 years ago

Huh, that's going to be fun in the end :see_no_evil: Sorry for turning everything upside down!

wincowgerDEV commented 3 years ago

It shouldn't be too much trouble :).

Warm Regards, Win

zsteinmetz commented 3 years ago

That dplyr::filter vs. signal::filter clash really troubles me .. still researching how to best circumvent it.

zsteinmetz commented 3 years ago

prospectr::savitzkyGolay works essentially the same but the length of the output vector is smaller than the original one. Would this get us into trouble?

zsteinmetz commented 3 years ago

The alternative would be to reimplement signal::filter. I digged into it, it's not so complicated. Let me know what you think. I call it a day.

wincowgerDEV commented 3 years ago

It troubles me too. I think we should probably just implement the SG filter as a function in the OS package that way we can call it what we want.

Warm Regards, Win

On Fri, Mar 5, 2021 at 9:58 AM Zacharias Steinmetz notifications@github.com wrote:

The alternative would be to reimplement signal::filter. I digged into it, it's not so complicated. Let me know what you think. I call it a day.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/zsteinmetz/OpenSpecy/issues/6#issuecomment-791584473, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGMUJU7BNNGI5QVEJOGQAYLTCELTJANCNFSM4YTMHWVQ .

--

´¯·.¸¸.·´¯·.´¯·.¸¸.·´¯ツ ------------------------------

Win Cowger PhD Candidate, Environmental Sciences: Soil and Water University of California, Riverside

NSF Graduate Research Fellow Research Advisor to 5 Gyres https://www.5gyres.org/advisors/ Data Advisor to Let's Do It World https://www.letsdoitworld.org/

Contact Info

515-298-3869 | wincowger@gmail.com

Websites www.openspecy.org www.wincowger.com http://andrewgray.ucr.edu/people/wcowger.html


zsteinmetz commented 3 years ago

I took a different approach for now: replacing dplyr::filter with subset.

wincowgerDEV commented 3 years ago

Sounds good, whatever works!

On Sat, Mar 6, 2021, 12:38 AM Zacharias Steinmetz notifications@github.com wrote:

I took a different approach for now: replacing dplyr::filter with subset.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/zsteinmetz/OpenSpecy/issues/6#issuecomment-791896503, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGMUJU3WASJIYSF2YDNWFRLTCHSYFANCNFSM4YTMHWVQ .

zsteinmetz commented 3 years ago

I guess, we're done here for now.