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

Use external dac codes #71

Merged
merged 6 commits into from
Jul 3, 2024
Merged

Use external dac codes #71

merged 6 commits into from
Jul 3, 2024

Conversation

fretchen
Copy link
Collaborator

@fretchen fretchen commented Jun 27, 2024

This is a first step to test the compatibility of the lists.

Quite interestingly it is directly running into troubles.

Approach

The DAC codes are pull from this source.
Then we look within the template for all the existing codes.

1. Issue: Missing codes

There is a substantial amount of codes that are missing. Some examples of missing codes:

the code 11321 is not present in the template
the code 11322 is not present in the template
the code 15120 is not present in the template
the code 15140 is not present in the template
the code 15161 is not present in the template
the code 15162 is not present in the template
the code 15163 is not present in the template
the code 15164 is not present in the template

2. Issue: Voluntary codes

The row voluntary codes is only present within the template. But no row like this present within the official reference.

@Maja4Dev any idea on what might be going on here ?

@fretchen fretchen linked an issue Jun 27, 2024 that may be closed by this pull request
2 tasks
@Maja4Dev
Copy link
Collaborator

Regarding the codes from the original DAC source you cited in your comment above @fretchen: all the codes mentioned by you are marked as "withdrawn" there - which means they have basically been eliminated from the original source and we dont need them either.

Regarding the DAC codes marked as "voluntary codes" in our template -> they are all part of the he original DAC sources you cited, only without being tagged as "voluntary". I guess this was maybe voluntary some time ago. For our purposes, we don't need this "voluntary" tag, so we can just make reference to this source.

@fretchen fretchen marked this pull request as ready for review June 28, 2024 17:10
@fretchen
Copy link
Collaborator Author

@Maja4Dev your comments fixed the problem indeed. So as a summary of this PR:

  • The list of sector codes is not validated against the official list.
  • The tests are run automatically on each PR and any change to main to make sure that the template does not break.
  • I made versionning of the required python libs easier using poetry.
  • I documented within the technical_notes.md which reference we are using.
  • I had to fix a few broken links within the technical_notes.md.

An open question for the future is if we would like to go the next step and fully generate the worksheet from the external json. I do not see this as high priority so I would propose to park it for the moment...

Looking forward to your comments @Maja4Dev.

@fretchen fretchen requested a review from Maja4Dev June 29, 2024 04:24
@Maja4Dev Maja4Dev merged commit 27a471e into main Jul 3, 2024
3 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
Development

Successfully merging this pull request may close these issues.

Use external DAC codes
2 participants