-
Notifications
You must be signed in to change notification settings - Fork 2
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
1031 split tests #1039
base: mar25-update
Are you sure you want to change the base?
1031 split tests #1039
Conversation
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view, the 📜action log, or 📝 job summary for details.Unrecognized words (2)processs These words are not needed and should be removedanomymous datas scoial spssTo accept these unrecognized words as correct and remove the previously acknowledged and now absent words, you could run the following commands... in a clone of the [email protected]:Public-Health-Scotland/source-linkage-files.git repository curl -s -S -L 'https://raw.githubusercontent.com/check-spelling/check-spelling/main/apply.pl' |
perl - 'https://github.com/Public-Health-Scotland/source-linkage-files/actions/runs/12480701565/attempts/1'
OR To have the bot accept them for you, comment in the PR quoting the following line: Available 📚 dictionaries could cover words (expected and unrecognized) not in the 📘 dictionaryThis includes both expected items (313) from .github/actions/spelling/expect.txt and unrecognized words (2)
Consider adding them (in with:
extra_dictionaries: |
cspell:fullstack/dict/fullstack.txt
cspell:k8s/dict/k8s.txt
cspell:php/dict/php.txt
cspell:node/dict/node.txt
cspell:npm/dict/npm.txt To stop checking additional dictionaries, add (in check_extra_dictionaries: '' Errors (1)See the 📂 files view, the 📜action log, or 📝 job summary for details.
See ❌ Event descriptions for more information. If the flagged items are 🤯 false positivesIf items relate to a ...
|
if (check_year_valid(year, "sds")) { | ||
data <- data %>% | ||
slfhelper::get_chi() | ||
|
||
old_data <- get_existing_data_for_tests(data) | ||
old_data <- get_existing_data_for_tests(data) | ||
|
||
data <- rename_hscp(data) | ||
data <- rename_hscp(data) | ||
|
||
comparison <- produce_test_comparison( | ||
old_data = produce_source_sds_tests(old_data), | ||
new_data = produce_source_sds_tests(data) | ||
) %>% | ||
write_tests_xlsx(sheet_name = "sds", year, workbook_name = "extract") | ||
comparison <- produce_test_comparison( | ||
old_data = produce_source_sds_tests(old_data), | ||
new_data = produce_source_sds_tests(data) | ||
) %>% | ||
write_tests_xlsx(sheet_name = "sds", year, workbook_name = "extract") | ||
|
||
return(comparison) | ||
return(comparison) | ||
} else { | ||
return(NULL) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree with this. Looks fine
Hi Zihao, i have had a look through your changes. I think we can have a discussion about this when you are back from leave. I have a few questions:
Happy to have a discussion about this when you are back, Thanks :) |
This branch writes out tests into different xlsx files separately to avoid writing into one file simultaneously and then combine the tests outputs into files by years. So, the final outputs are the same as before.
There is another branch 1031-remove-tests-from-targets, which takes out all tests from targets and do tests by sequence. Finishing all tests for each year would take around 25 minutes. It is time-consuming and hence this approach is much less favourable.
Summary of key changes:
setup_tests_file_name
: It is taken out fromwrite_tests_xlsx
and set up paths for tests file and folder.combine_multi_xlsx
,combine_extracts_tests_year
,combine_lookup_tests
,combine_tests
: Combine xlsx files togethertargets
script has been restore for parallel computing for tests and there is no problem of writing tests into one file simultaneously.processs_tests_sds
has been added filter ofcheck_year_valid
to avoid corruption intargets
fortests_sds_2425
.A thorough test has been done and successfully went through without any problem. So it is good for review and then merge.