search

ncss-tech / soilDB

soilDB: Simplified Access to National Cooperative Soil Survey Databases
http://ncss-tech.github.io/soilDB/
GNU General Public License v3.0
81 stars 20 forks source link

get_RMF_from_NASIS_db() + uncode() may be shifting Munsell chroma with user-defined dsn #356

Open dylanbeaudette opened 1 week ago

dylanbeaudette commented 1 week ago

Need to check more closely, but it seems like Munsell chroma are shifted-down 1 unit with a user-defined dsn argument pointing to a local SQLite database containing plaintext vs. coded values.

brownag commented 1 week ago

If I understand correctly, you have created a SQLite database with plaintext values, so I think this is "expected" since uncode() is always called in get_RMF_from_NASIS_db()

You could try and set options(soilDB.NASIS.skip_uncode = TRUE) to bypass decoding logic from uncode(). This should work for "simple" functions like get_RMF_from_NASIS_db() that are just basic wrappers around a query (that does not rely on decoded values for post-processing).

createStaticNASIS() does not attempt to decode values to ensure compatibility/parity with NASIS access via local ODBC connection to SQL Express server. I can understand why you would want to put decoded values in SQLite DB, which is why above option was created. It is mentioned in the uncode() documentation but probably could be advertised a bit better.

dylanbeaudette commented 1 week ago

Thanks, this is exactly what I was missing--an option to disable automatic use of uncode(). I'll review the documentation and see what I can add to clarify for my future self.

The SQLite DB in question is a NASIS pedon "snapshot", produced from uncoded values exclusively. I was curious to see if I could point fetchNASIS() and related functions at this thing. It mostly works. Now that I know how to disable uncode() I can proceed with further testing.

brownag commented 1 week ago

As I recall the NASIS pedon snapshot uses the domain "label" rather than the "name"? If so there might be a couple routines that are not robust to that, due to case sensitivity etc., but they are few and far between. fetchNASIS() should work, for the most part, on that, but a couple of the subroutines that do aggregation of many:1 might not work depending on how values are coded.

dylanbeaudette commented 1 week ago

I'll ask Adolfo, he has been making these for the last couple of years. I'd like to know if this will work in practice, even if it is a rare task. fetchNASIS() got about half-way through before barfing with an error related to negative indexing on a data.frame. I'll do some more testing once I get a more modern snapshot. The most recent one that I have is from Sept. 2021.

dylanbeaudette commented 1 week ago

So, last TODO here

brownag commented 1 week ago

I'll ask Adolfo, he has been making these for the last couple of years. I'd like to know if this will work in practice, even if it is a rare task. fetchNASIS() got about half-way through before barfing with an error related to negative indexing on a data.frame. I'll do some more testing once I get a more modern snapshot. The most recent one that I have is from Sept. 2021.

I believe I have run this at some time in the past on the exact same snapshot. I was not aware of a more recent version. Possibly the error is a relatively newer bug that was introduced?

dylanbeaudette commented 1 week ago

Could be. I'll put the latest version (once I get it) on the office server for testing.