Skip to content
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

if inputdata files are missing, try to download them. Add test. #1577

Merged
merged 4 commits into from
Feb 27, 2024

Conversation

orichters
Copy link
Contributor

@orichters orichters commented Feb 26, 2024

Purpose of this PR

Current situation:

This PR:

  • checks whether all input files are present as desired
  • if missing, download and distribute input data. But don't do that while running --gamscompile to avoid that it tries to fix that over and over.
  • add a test such that make test checks whether input data is complete
  • suppress most input data update messages while running --gamscompile to avoid clutter in the terminal
  • requires add 'path' param to getfiledestinations pik-piam/gms#88, new function missingInputData and some changes to start scripts to allow them being called from testthat directory
  • The ECEMF config files passes the gamscompile test now, so need not be skipped anymore. Comment out all pseudo-comment scenarios to speed up the test

A failing test looks like that:

✖ | 1 2      0 | 01-inputdata [24.6s]
─────────────────────────────────────────────────
Warning (test_01-inputdata.R:5:5): Are all input data files present?
File olistest.csv seems to be missing!
Backtrace:
    ▆
 1. └─global updateInputData(cfg = gms::readDefaultConfig("../.."), remindPath = "../..") at test_01-inputdata.R:5:5
 2.   └─gms::download_distribute(...)
 3.     └─gms::copy_input(...) at gms/R/download_distribute.R:115:5

Warning (test_01-inputdata.R:9:5): Are all input data files present?
Missing input files: core/input/olistest.csv

Failure (test_01-inputdata.R:11:3): Are all input data files present?
length(missinginput) == 0 is not TRUE

`actual`:   FALSE
`expected`: TRUE

Type of change

  • New feature
  • No impact on scenarios

Checklist:

  • I performed a self-review of my own code
  • I explained my changes within the PR, particularly in hard-to-understand areas
  • All automated model tests pass (FAIL 0 in the output of make test): [ FAIL 0 | WARN 0 | SKIP 6 | PASS 82 ]
  • The changelog CHANGELOG.md has been updated correctly

@orichters
Copy link
Contributor Author

tests still work: [ FAIL 0 | WARN 0 | SKIP 6 | PASS 82 ]

Copy link
Member

@LaviniaBaumstark LaviniaBaumstark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make test is working including the new test, right?

f31_extraseed.cs4r
*f31_decoffset.cs4r
*f31_Xport.cs4r
*f31_extraseed.cs4r
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe delete those lines?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, as long as they are still called here, I think it is better just to comment them out:

$include "./modules/31_fossil/timeDepGrades/input/f31_decoffset.cs4r"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but they do not exist...and if yomeone would provide them, they would not be dirstributed as long as they would be commented in in this file. So, I think, it is better to have no outcommented code. If this will be reactivated, there are anyway code adjsutments necessary.

@orichters
Copy link
Contributor Author

make test is working including the new test, right?

yes.

@orichters orichters merged commit d4d2b3d into remindmodel:develop Feb 27, 2024
2 checks passed
@orichters orichters deleted the fixinput branch March 18, 2024 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants