-
Notifications
You must be signed in to change notification settings - Fork 1
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
#695 Move Code List SPARQL Queries to CodeListInspector #725
Conversation
ubuntu-latest-python3.9 test results460 tests +4 460 ✔️ +4 2m 59s ⏱️ -16s Results for commit 2e5142e. ± Comparison against base commit 9bea1bd. This pull request removes 1 and adds 5 tests. Note that renamed tests count towards both.
♻️ This comment has been updated with latest results. |
ubuntu-latest-python3.11 test results460 tests +4 460 ✔️ +4 2m 49s ⏱️ -15s Results for commit 2e5142e. ± Comparison against base commit 9bea1bd. This pull request removes 1 and adds 5 tests. Note that renamed tests count towards both.
♻️ This comment has been updated with latest results. |
windows-latest-python3.11 test results 9 files ±0 10 suites ±0 7m 53s ⏱️ +17s Results for commit 2e5142e. ± Comparison against base commit 9bea1bd. This pull request removes 1 and adds 5 tests. Note that renamed tests count towards both.
♻️ This comment has been updated with latest results. |
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.
Looks good, makes sense to me! There are a few differences in the way cached properties are implemented compared to how we did it in the other tickets with myself and Sarah (data_cube_state or csvw_state), but I think these cached properties mostly work independently between code lists and data cubes. There will be quite a bit of conflict resolution needed when we merge all these 3 cached properties tickets together, but that's a natural part of splitting up the task the way we did. I don't see anything here that I think we wouldn't want to keep anyway.
tests/test-cases/cli/inspect/itis-industry-no-skos-inscheme.csv-metadata.json
Outdated
Show resolved
Hide resolved
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.
Almost there, just tidy up those comments and I think we're pretty much there.
Kudos, SonarCloud Quality Gate passed! |
def get_column_definitions_for_csv(self, csv_url: str) -> List[ColumnDefinition]: | ||
""" | ||
Returns the `ColumnDefinition`s for a given csv file, raises a KeyError if the csv_url | ||
is not associated with a ColumnDefinition. |
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.
A list of column definitions.
#695
transferred the following queries to cached properties:
Adds a get_catalog_metadata API function to code_list_inspector.py which uses the results from the select_codelist_csv_url.sparql cached property.
Adds unit test coverage for new functions/properties
Class has been renamed to CodeListInspector