Use lib360dataquality for validation
#65
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
|
This is great! Thanks, No problems with the code.. There is one issue that it's raised which is that each invocation of the validation will download the schema and codelists from github instead of the locally held ones. I think there is a way around this but needs a bit of thinking |
Member
|
So I'm thinking in lib360dataquality we could modify it to take the file paths that are cached in the datagetter https://github.com/ThreeSixtyGiving/datagetter/blob/tw/lib360dataquality/getter/get.py#L394 and pass them to init of e.g. diff --git a/lib360dataquality/cove/schema.py b/lib360dataquality/cove/schema.py
index e560618..5ea54c8 100644
--- a/lib360dataquality/cove/schema.py
+++ b/lib360dataquality/cove/schema.py
@@ -28,7 +28,11 @@ class Schema360(SchemaJsonMixin):
_pkg_schema_obj = {}
_schema_obj = {}
- def __init__(self, data_dir) -> None:
+ def __init__(self, data_dir, local_pkg_schema_path=None, local_grant_schema_path=None) -> None:
+ """ data_dir: The directory with the data
+ local_pkg_schema: If specified will use the path provided instead of fetching from remote
+ local_grant_schema_path: If specified will use the path provided instead of fetching from remote
+ """
# Create dedicated location for schema work
self.working_dir = os.path.join(data_dir, "schema")
try:
@@ -36,6 +40,17 @@ class Schema360(SchemaJsonMixin):
except FileExistsError:
pass
+ if local_pkg_schema_path:
+ # copy schema to working dir
+ # setup self._pkg_schema_obj
+
+ if local_grant_schema_path:
+ # copy schema to woring dir
+ # setup self._schema_obj
+
+ # Don't do the remote fetching stuff if local^
+
+
# required by lib-cove for CustomRefResolver the trailing / is needed to make sure
# urljoin does not discard the final part of the location.
self.schema_host = f"{self.working_dir}/"I'm also wondering if there is more scope to de-duplicate some code as lib360dataquality also has the convert/unflatten functions (e.g. from xlsx -> json), though that's not a priority here it might be useful context. |
Member
|
For testing debugging of the requests I was doing: diff --git a/getter/get.py b/getter/get.py
index b8c3560..87b815e 100644
--- a/getter/get.py
+++ b/getter/get.py
@@ -18,6 +18,26 @@ from lib360dataquality.cove.schema import Schema360
from lib360dataquality.cove.threesixtygiving import common_checks_360
import getter.cache as cache
+import logging
+
+# These two lines enable debugging at httplib level (requests->urllib3->http.client)
+# You will see the REQUEST, including HEADERS and DATA, and RESPONSE with HEADERS but without DATA.
+# The only thing missing will be the response.body which is not logged.
+try:
+ import http.client as http_client
+except ImportError:
+ # Python 2
+ import httplib as http_client
+http_client.HTTPConnection.debuglevel = 1
+
+# You must initialize logging, otherwise you'll not see debug output.
+logging.basicConfig()
+logging.getLogger().setLevel(logging.DEBUG)
+requests_log = logging.getLogger("requests.packages.urllib3")
+requests_log.setLevel(logging.DEBUG)
+requests_log.propagate = True
+
+
acceptable_licenses = [
"http://www.opendefinition.org/licenses/odc-pddl",
"https://creativecommons.org/publicdomain/zero/1.0/", |
michaelwood
requested changes
Mar 25, 2025
Member
michaelwood
left a comment
michaelwood
left a comment
There was a problem hiding this comment.
We need to sort out the issue in lib360dataquality
Contributor
Author
|
@michaelwood just to check whether I understand, is the problem that on initialising |
Member
|
@tillywoodfield Yes that's correct. It would be ideal to only initialise |
Member
|
Just having a go at also using convert_spreadsheet from lib360dataquality to further reduce the duplicated code. Also updated the implementation for validate to use the work in ThreeSixtyGiving/dataquality#148 |
Member
|
note for self the path isn't being set correctly: Also need to make sure that tests do not use caching |
This simplifies the flow of `get` by removing options branches that cause the code to be difficult to read.
This will be used when testing to make sure any caches are removed between test runs.
This reduces any code duplication between the DQT and the datagetter which may result in differing behaviours. This also adds support for extensions.
michaelwood
force-pushed
the
tw/lib360dataquality
branch
from
073b3ec to
44ff207
Compare
michaelwood
requested review from
R2ZER0 and
michaelwood
and removed request for
michaelwood
michaelwood
approved these changes
Apr 17, 2025
R2ZER0
approved these changes
Apr 23, 2025
Contributor
R2ZER0
left a comment
R2ZER0
left a comment
There was a problem hiding this comment.
I'm not especially familiar this code yet but overall looks good 👍
Contributor
Author
|
I'm going to close this PR as part of my wrapping up but the branch is still there if you want to open another one :) |
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #57