PEcAn.DB and PEcAn.meta.analysis cleanup
#2351
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Also, add `rjags` as a required dependency of `PEcAn.MA`, and add to Travis.
Also, clean up and update documentation. Related to PecanProject#1747.
Because it doesn't require prepared statements and therefore the "Postgres" driver.
All `PEcAn.DB` tests should start with.
```r
source("db.setup.R")
con <- check_db_test()
```
This helps prevent redundancy in database connection setup, and makes
it easier to accommodate different database configurations in tests
down the line.
Also, note that many of these tests would fail when switching from
`PostgreSQL` to `Postgres` because the latter uses `integer64` for
long integers, while the former uses `double`. These changes force
conversion to numeric in all comparisons.
12 tasks
|
@ashiklom Want me to assign a reviewer on this or do you have someone in mind? |
|
@tonygardella @ashiklom I started reviewing #2358, which is a superset of this one, and haven't yet finished. Would it be useful for me to finish reading there and then copy my approval here? |
|
Closing this because it has been entirely superceded by #2358 (for which @infotroph gave a detailed and very helpful review -- thanks!). |
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PEcAn.DB::get.trait.data, iftrait.namesisNULLor missing, use the traits for which at least one prior is available among the input list of PFTs. (Previously, we were getting this from thePEcAn.utils::trait.dictionary, which we are trying to deprecate remove base/utils/data/trait.dictionary.csv #1747 ).PEcAn.DB::query_priorsthat is, IMHO, more robust and intuitive thanquery.priorsby leveragingRPostgresprepared statements, providing more informative errors, and handling inputs in a more sophisticated way. Its output should be a perfect superset ofquery.traits, so I think it should work as a drop-in replacement. Note that a unit test and detailed function documentation are included.package::function) throughoutPEcAn.meta.analysis. Otherwise, many of these functions would fail when trying to run a meta-analysis outside of the PEcAn workflow (i.e. without having loaded the packages first).PEcAn.meta.analysisandPEcAn.DB.Review Time Estimate
Types of changes
Checklist: