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

codeCheck: edit not_used.txt only if interactive; auto-remove stuff never declared #92

Merged
merged 3 commits into from
Mar 18, 2024
Merged

Conversation

orichters
Copy link
Contributor

@orichters orichters commented Mar 8, 2024

Example

What happens if you add cm_NDC_version, which is a switch name but has no GAMS definition, to modules/46_carbonpriceRegi/netZero/not_used.txt in remindmodel?

In the following, irrelevant stuff shortened with [...]:

Old with interactive = FALSE. While the first warning makes sense, the three others are completely misleading.

 Running codeCheck...
[...]
 Switch Appearance check done...        (time elapsed:  70.92)
 Interface collection and check done... (time elapsed:  71.07)
 Input folder check done...             (time elapsed:  71.08)
 Description check done...              (time elapsed:  71.08)
Error in `gms::codeCheck()`:
! codeCheck returned warnings. Fix warnings to proceed!
Backtrace:
    ▆
 1. └─gms::codeCheck(strict = TRUE)
Warning messages:
1: Could not find any declaration for cm_NDC_version
2: 'cm_NDC_version' is not addressed in all realizations of module 'carbonprice'! (diffCurvPhaseIn2Lin=0, exogenous=0, expoLinear=0, exponential=0, linear=0, NDC=1, none=0, NPi=0,
temperatureNotToExceed=0) (0 = missing, 1 = in code, 2 = in not_used.txt)
3: 'cm_NDC_version' is not addressed in all realizations of module 'carbonpriceRegi'! (NDC=1, netZero=2, none=0) (0 = missing, 1 = in code, 2 = in not_used.txt)
4: 'cm_NDC_version' is not addressed in all realizations of module 'techpol'! (coalPhaseout=0, coalPhaseoutRegional=0, CombLowCandCoalPO=0, lowCarbonPush=0, NDC=1, NDCplus=1, NPi2018=1,
none=0) (0 = missing, 1 = in code, 2 = in not_used.txt)
Execution halted
make: *** [Makefile:67: check] Error 1

Old with interactive = TRUE. Asks many questions, if you accept them it adds the switch to many not_used.txt where they don't belong, and then at the end finally tells you the actual problem.

 Running codeCheck...
[...]
 Switch Appearance check done...        (time elapsed:  71.05)
In module 'carbonprice', 'cm_NDC_version' is not addressed in those 8 realizations: diffCurvPhaseIn2Lin, exogenous, expoLinear, exponential, linear, none, NPi, temperatureNotToExceed.
If you are sure that it does not need to be addressed at all you can add this to 'not_used.txt'.
Type the reason why it does not need to be addressed, or type 'n' if it actually needs to be addressed in the code.
whatever reason
In module 'carbonpriceRegi', 'cm_NDC_version' is not addressed in those 1 realizations: none.
If you are sure that it does not need to be addressed at all you can add this to 'not_used.txt'.
Type the reason why it does not need to be addressed, or type 'n' if it actually needs to be addressed in the code.
whatever reason
In module 'techpol', 'cm_NDC_version' is not addressed in those 5 realizations: coalPhaseout, coalPhaseoutRegional, CombLowCandCoalPO, lowCarbonPush, none.
If you are sure that it does not need to be addressed at all you can add this to 'not_used.txt'.
Type the reason why it does not need to be addressed, or type 'n' if it actually needs to be addressed in the code.
whatever reason
 Interface collection and check done... (time elapsed: 110.71)
 Input folder check done...             (time elapsed: 110.72)
 Description check done...              (time elapsed: 110.72)
Error in `gms::codeCheck()`:
! codeCheck returned warnings. Fix warnings to proceed!
Backtrace:
    ▆
 1. └─gms::codeCheck(strict = TRUE, interactive = TRUE)
Warning message:
Could not find any declaration for cm_NDC_version
Execution halted
make: *** [Makefile:72: check-fix] Error 1

New with interactive = FALSE: Just one straight warning.

 Running codeCheck...
[...]
 Switch Appearance check done...        (time elapsed:  73.59)
 Interface collection and check done... (time elapsed:  73.71)
 Input folder check done...             (time elapsed:  73.72)
 Description check done...              (time elapsed:  73.72)
Error in `gms::codeCheck()`:
! codeCheck returned warnings. Fix warnings to proceed!
Backtrace:
    ▆
 1. └─gms::codeCheck(strict = TRUE)
Warning message:
cm_NDC_version was never declared, but exists only in not_used.txt of carbonpriceRegi
Execution halted
make: *** [Makefile:67: check] Error 1

New with interactive = TRUE. As the case is obvious, doesn't even ask.

 Running codeCheck...
[...]
 Switch Appearance check done...        (time elapsed:  67.26)
 Cleanup not_used.txt...                (time elapsed:  67.34)
'cm_NDC_version' has been removed as interface and has been deleted from all 3 not_used.txt files of module 'carbonpriceRegi'!

 Interface collection and check done... (time elapsed:  67.46)
 Input folder check done...             (time elapsed:  67.47)
 Description check done...              (time elapsed:  67.47)
 All codeCheck tests passed!

writeLines(c(comment, paste(colnames(tmp), collapse = ",")), n)
write.table(tmp, n, sep = ",", quote = FALSE, row.names = FALSE, append = TRUE, col.names = FALSE)
}
if (v %in% rownames(ap$appearance)) ap$appearance[v, r] <- 0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the if makes sure that stuff that was removed above in line 210 does not fail this line.

@orichters orichters marked this pull request as draft March 8, 2024 17:35
@orichters orichters marked this pull request as ready for review March 8, 2024 17:40
Copy link

codecov bot commented Mar 8, 2024

Codecov Report

Attention: Patch coverage is 12.90323% with 27 lines in your changes are missing coverage. Please review.

Project coverage is 41.47%. Comparing base (9a15a92) to head (1bb2a63).

Files Patch % Lines
R/codeCheck.R 12.90% 27 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master      #92      +/-   ##
==========================================
- Coverage   41.60%   41.47%   -0.13%     
==========================================
  Files          51       51              
  Lines        1709     1719      +10     
==========================================
+ Hits          711      713       +2     
- Misses        998     1006       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@orichters orichters linked an issue Mar 14, 2024 that may be closed by this pull request
Copy link
Member

@tscheypidi tscheypidi left a comment

Choose a reason for hiding this comment

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

looks good

@orichters orichters merged commit e9a8b9a into pik-piam:master Mar 18, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants