Conversation
SFJohnson24
left a comment
SFJohnson24
left a comment
There was a problem hiding this comment.
This looks good so far-- can you see messages on slack with me and Gerry re: the .env and taking that into account as well with csv runs. https://github.com/cdisc-org/cdisc-rules-engine/issues/1559 My parsing script matches your reader logic--there is a description of the .env structure there.
It would want to get the standard/version/substandard/CT/define xml path from the .env
…-dp, -d] parameters to run validation.
| raise NotImplementedError | ||
|
|
||
| def from_file(self, file_path): | ||
| with open(file_path, "r", encoding=self.encoding) as fp: |
There was a problem hiding this comment.
We will need encoding + general error handling wrapping similar to json reader here.
| first_record = {} | ||
|
|
||
| with open(self.file_path, encoding=self.encoding) as f: | ||
| dataset_length = sum(1 for _ in f) - 1 # subtract header |
There was a problem hiding this comment.
This can result in dataset_length = -1 for empty csv files. For those cases I guess it should be zero. So we should clamp it to zero.
| description = "JSON data is malformed." | ||
|
|
||
|
|
||
| class InvalidCSVFormat(EngineError): |
There was a problem hiding this comment.
I think better naming for this would be InvalidCSVFile instead of format because this error is for when csv is malformed or encoding failed which makes it invalid csv file. What do you think?
There was a problem hiding this comment.
hey @alexfurmenkov can you please start adding some descriptions to your PRs? It would help with the reviews. Thanks
I tried running a CSV with no standard / version given per the discussion in slack and core.py still has standard and version as required for their click arguments. This needs to be changed. I tried changing them myself locally and it still fails-- it appears it is not grabbing them from the env.
File "C:\Users\SamJohnson\Documents\workspaces\cdisc-rules-engine/core.py", line 537, in validate
standard = standard.lower()
it seems the click strategy you implemented isnt grabbing things because envvar only works in the process environment and the .env we want is in the -d path.
note from the slack screenshot above: --use-case also needs to be implemented and extracted from the env as well
# Conflicts: # cdisc_rules_engine/exceptions/custom_exceptions.py
3 participants
No description provided.