Skip to content

Hotfix/additional invalid issues - #854

Open
alex-silverman wants to merge 159 commits into
devfrom
hotfix/additional-invalid-issues
Open

Hotfix/additional invalid issues#854
alex-silverman wants to merge 159 commits into
devfrom
hotfix/additional-invalid-issues

Conversation

@alex-silverman

@alex-silverman alex-silverman commented Oct 27, 2025

Copy link
Copy Markdown
Collaborator

Using machine specs to detect invalid values across all columns in all files.

Alex Silverman added 3 commits September 22, 2025 16:06
…ed on FY26 specs. And smal updates to this fuller check to handle special columns and beginnings of nullunless check
@alex-silverman
alex-silverman marked this pull request as draft October 30, 2025 13:13
@alex-silverman
alex-silverman marked this pull request as ready for review November 4, 2025 16:01
@alex-silverman
alex-silverman marked this pull request as draft November 4, 2025 16:02
@alex-silverman
alex-silverman deleted the branch dev November 17, 2025 15:35
@alex-silverman
alex-silverman changed the base branch from feat/monthly-inflow-outflows to dev November 17, 2025 15:47
alex-silverman and others added 15 commits January 23, 2026 16:38
…nd more automatically parse the specs (after updating it)
…ng session data. Whether we actually store them will depend on whether they're in the Export.csv file
…e old cols_and_data-types (now called column_priorities) now just provides Eva's DataTypeHighPriority indicator.

Switching to the specs doc, though, means we have to update references to the File and Column columns, which are now named CSV and Name, respectively.

also using an updated data_type_mapping to map the data types referenced in the specs with the R Classes of the columns of the read-in csvs.
…to the detect_invalid_values script, since they're all based on the specs doc now. Also removing the ExportID inconsistency, since that's captured by the Relational Integrity Violation check
@alex-silverman
alex-silverman marked this pull request as ready for review July 6, 2026 15:11
Alex Silverman added 2 commits July 10, 2026 09:32
Merge branch 'dev' into hotfix/additional-invalid-issues

# Conflicts:
#	tests/helper_data/main-valid/dq_main.csv
#	tests/testthat/_snaps/linux-4.5/main-valid/test-main-valid-exportTestValues.json
@alex-silverman

Copy link
Copy Markdown
Collaborator Author

cols_and_data_types

i think this is resolved? I set cols_and_data_types in the TRUE case now.

@alex-silverman

Copy link
Copy Markdown
Collaborator Author

unique_id_lookup

I think this is no longer a problem:

  • unique_id_lookup is now joined onto reporting_info, so we don't need it anywhere else
  • the other 2 are defined in the TRUE case (where the file already existS)

@mlfurman3

Copy link
Copy Markdown
Collaborator

Overall the changes are looking much improved and working for most scenarios.

Found one modified dataset that still broke the upload, in demo dataset mods dir: demo_FSACheckMods_HPFSA,zip

@vlopez0603

Copy link
Copy Markdown
Collaborator

In reviewing, noticed that DQ and PDDE checks for "Nulls not allowed" and "Invalid Non-Null Value" were appearing as mostly empty rows in the PDDE And DQ reports.

image image

@mlfurman3

Copy link
Copy Markdown
Collaborator

@alex-silverman I did some digging based on the above comment, and it seems like these two types of checks, along with others, do not have AnchorIDs in specs_rules that they reference to pull in the correct info.

What is the reasoning for setting up some checks with no AnchorID?

@alex-silverman

Copy link
Copy Markdown
Collaborator Author

demo_FSACheckMods_HPFSA

Nice catch. Just pushed a fix (exiting out early from part of the code if any Key Fields are not in the dataset).

@alex-silverman

alex-silverman commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

@alex-silverman I did some digging based on the above comment, and it seems like these two types of checks, along with others, do not have AnchorIDs in specs_rules that they reference to pull in the correct info.

What is the reasoning for setting up some checks with no AnchorID?

@vlopez0603 - which dataset did you use and which tab were you looking at? I just tried DEMO mode and didn't see a blank org in any of the HP, Error, or Warning tabs.
@mlfurman3 - I think Victoria filled out the indicator in the specs spreadsheet for whether to include Anchor IDs or not. For some files, like Users.csv, there is no Anchor ID. But I don't remember why the other fields don't have one.

@alex-silverman

Copy link
Copy Markdown
Collaborator Author

@alex-silverman I did some digging based on the above comment, and it seems like these two types of checks, along with others, do not have AnchorIDs in specs_rules that they reference to pull in the correct info.
What is the reasoning for setting up some checks with no AnchorID?

@vlopez0603 - which dataset did you use and which tab were you looking at? I just tried DEMO mode and didn't see a blank org in any of the HP, Error, or Warning tabs. @mlfurman3 - I think Victoria filled out the indicator in the specs spreadsheet for whether to include Anchor IDs or not. For some files, like Users.csv, there is no Anchor ID. But I don't remember why the other fields don't have one.

@vlopez0603 - I think the specs dataset hadn't been updated from my dev version to the one on eva-dev. Can you try again?

@vlopez0603

Copy link
Copy Markdown
Collaborator

Most of the blank rows have been resolved though there's still one on the PDDE side (see screenshot below). This case is actually flagging a missing Geocode for a project.

image

I think it may be on me to update the specs sheet on ACE for at least one field. I may have missed indicating to include the AnchorID for that data element/column.

@vlopez0603

Copy link
Copy Markdown
Collaborator

@alex-silverman / @mlfurman3 - I made minor updates to the specs spreadsheet. I noticed the Incorrect Data Type check had three columns incorrectly set as PDDE instead of FSA. The Non-Null Invalid check also had one column incorrectly set as PDDE instead of FSA.

I'm still in the process of reviewing the other checks in the specs spreadsheet.

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.

3 participants