RMI-PACTA / r2dii.match

Tools to Match Financial Portfolios with Climate Data
https://rmi-pacta.github.io/r2dii.match
Other
7 stars 5 forks source link

upkeep: bump r2dii version in `helper-skip_if_r2dii_data_outdated.R` #476

Closed jdhoffa closed 6 months ago

jdhoffa commented 6 months ago

The (brittle) snapshot tests are currently failing when run against the CRAN published r2dii.data v0.5.0. They only pass against the dev version of the package.

This PR bumps the function that skips snapshot tests with old r2dii.data.

Another indication that we should prioritize working on #392

Relates to https://github.com/RMI-PACTA/r2dii.data/pull/374 Relates to https://github.com/RMI-PACTA/r2dii.match/pull/473

jdhoffa commented 6 months ago

Hmm ok that didn't seem to work actually. Let me see what is going on

jdhoffa commented 6 months ago

@jacobvjk it turns out the tests that were failing were those that tests the expected names of the output of match_name.

Since we have changed the column names in the input data, the expected names of the output will be different between 0.5.0 and 0.5.0.9000 (and beyond).

There are two options:

of the two, I prefer the former option, since we have a stronger test of match_name, and all of this is dev-facing anyway.

jacobvjk commented 6 months ago

@jacobvjk it turns out the tests that were failing were those that tests the expected names of the output of match_name.

Since we have changed the column names in the input data, the expected names of the output will be different between 0.5.0 and 0.5.0.9000 (and beyond).

There are two options:

  • Skip the tests unless the r2dii.data version is sufficiently high OR
  • Soften the test to ensure that the output CONTAINS the expected names (rather than strictly expecting the EXACT names)

of the two, I prefer the former option, since we have a stronger test of match_name, and all of this is dev-facing anyway.

agree with this and definitely better to work on removing the skips by removing the test dependency on r2dii.data soon