-
Notifications
You must be signed in to change notification settings - Fork 20
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
fetchNASIS phlabresults join issue #192
Comments
Thanks for bringing this up @hammerly; some things were not working as they should. I made some changes in #195 that should address your immediate needs and work out-of-the-box for your two pedons. Now the original weighted averaging/dominant logic works as expected with the following notice for your example pedons:
If we allow the lab data to be joined in at the beginning of If you can please install off the https://github.com/ncss-tech/soilDB/tree/phlabresultswt branch and see if this resolves short term issues with remotes::install_github("ncss-tech/soilDB@phlabresultswt", dependencies=FALSE) Now here are some things I am thinking about regarding more detailed aggregation options. From your suggestions I see three proposed aggregation methods: "most recent", "weighted average" and "none"; and I support these ideas
|
Thanks @brownag the branch resolves the issue with the fetch erroring out. Your proposed aggregation methods would be excellent features to add if we can figure out and agree on a way to implement them. For the "most recent" method, you are right, the |
Thanks @hammerly! I wasn't considering the child tables; good point for at least some of the analytes there would be |
When using
fetchNASIS(from = "pedons", lab = TRUE)
on pedons which contains > 1 row in the phlabresults table, it results in a join error, and fails to fetch any results. In most cases, this is probably desired, as it prevents other issues, and may identify data quality issues. A small percentage of pedons however, do contain lab results for specific depths within a horizon, in these situations, it might be useful to still fetch them and potentially analyze separately, or combine the results as a weighted average. Another edge case happens if the results from lab analysis for the same horizon are different than expected, the scientist may run the soil sample through the lab analysis again and obtain different results for the same sample and add an additional row to the phlabresults table for that horizon. In this case, it might be appropriate to take the newest result. This issue is similar to the issue identified in #120 for the phsample table. For examples, load upedonid: 1964IA113001, 1975IA045012. A possible remedy to this issue would be to implement something similar to thermHzErrors=FALSE
argument but specifically for the phlabresults data. This issue was briefly discussed with @smroecker.The text was updated successfully, but these errors were encountered: