Skip to content

BYOC_ID: 40072 refactor-acute#1283

Open
OluwatobiOni wants to merge 30 commits into
developmentfrom
refactor-acute
Open

BYOC_ID: 40072 refactor-acute#1283
OluwatobiOni wants to merge 30 commits into
developmentfrom
refactor-acute

Conversation

@OluwatobiOni

Copy link
Copy Markdown
Collaborator

No description provided.

@OluwatobiOni
OluwatobiOni requested review from Jennit07 and lizihao-anu and removed request for lizihao-anu April 13, 2026 09:50
@OluwatobiOni

Copy link
Copy Markdown
Collaborator Author

Dependency missing: Acute CUP file

@Jennit07 Jennit07 added On hold Waiting for something / someone outside of our control BYOC 1: Dependency Data Not Available Dependency data not available in preprod 1a: Ready for Review 1 Refactoring completed by author (without data) labels Apr 13, 2026
@Jennit07 Jennit07 mentioned this pull request Apr 13, 2026
38 tasks
@Jennit07 Jennit07 self-assigned this Apr 13, 2026
@Jennit07 Jennit07 changed the title refactor-acute BYOC_ID: 40072 refactor-acute Apr 13, 2026
@lizihao-anu lizihao-anu linked an issue Apr 15, 2026 that may be closed by this pull request
Comment thread R/read_extract_acute.R Outdated
Comment thread R/process_extract_acute.R Outdated

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm getting the following error on this line:

Error in `dplyr::mutate()`:
ℹ In argument: `month_name = month_order[.data$costmonthnum]`.
Caused by error:
! `month_name` must be size 100 or 1, not 0.
Run `rlang::last_trace()` to see where the error occurred.
Called from: signal_abort(cnd, .file)

We think this is due to costmonthnum being an integer64 variable but UAT tests show this should be numeric. This has been flagged and should be updated in the next round of UAT feedback

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think it happened in refactoring some dataset before. I will have a look at get back to suggest changes to make it consistent.

@Jennit07 Jennit07 removed the 1a: Ready for Review 1 Refactoring completed by author (without data) label Apr 22, 2026
@Jennit07

Copy link
Copy Markdown
Collaborator

I have reviewed this PR and it looks good! there is an error i am getting which i have commented above. This should be corrected in the next round of UAT feedback.

Waiting on:

  • Acute CUP file
  • data types updated in UAT

This will be ready for BYOC testing once the two issues are addressed.

@OluwatobiOni OluwatobiOni added the 1a: Ready for Review 1 Refactoring completed by author (without data) label May 1, 2026
@Jennit07 Jennit07 added 2: Preprod Data in UAT Denodo data available in preprod and undergoing UAT ready for PHS testing 1b: Peer Review 1 Complete First peer review completed by reviewer (without data) and removed 1a: Ready for Review 1 Refactoring completed by author (without data) labels May 11, 2026
Co-authored-by: Jennifer Thom  <67372904+Jennit07@users.noreply.github.com>
@LucyEmma22
LucyEmma22 marked this pull request as ready for review June 30, 2026 14:46
@LucyEmma22 LucyEmma22 added On hold Waiting for something / someone outside of our control 1a: Ready for Review 1 Refactoring completed by author (without data) Requires UAT Changes 2a: Ready for Review 2 Local BYOC testing completed by author (with data) and removed 3a: Ready for BYOC Testing Local testing completed and reviewed, all data available and all UAT passed 1b: Peer Review 1 Complete First peer review completed by reviewer (without data) On hold Waiting for something / someone outside of our control 1a: Ready for Review 1 Refactoring completed by author (without data) labels Jun 30, 2026
c_year <- convert_fyyear_to_year(year)

# Specify years available for running
if (!check_year_valid(year, type = "acute_cup")) {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

are we updating check_year_valid to include acute_cup? Otherwise, we can just use "acute" as the type here since it is dependent on acute anyway. @LucyEmma22

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ooh that's a good spot. Maybe I'll add it to the check_year_valid type list for consistency?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Or should we just remove this check since we weren't previously checking valid years for the CUP files? @Jennit07

@LucyEmma22 LucyEmma22 added On hold Waiting for something / someone outside of our control 2: Dependency Data in UAT Dependency data available in preprod and undergoing UAT labels Jul 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2a: Ready for Review 2 Local BYOC testing completed by author (with data) 2: Dependency Data in UAT Dependency data available in preprod and undergoing UAT 2: Preprod Data in UAT Denodo data available in preprod and undergoing UAT BYOC On hold Waiting for something / someone outside of our control

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BYOC_ID: 40072 - Refactor Acute (SMR01 + GLS)

4 participants