Skip to content

Verify via tool - #92

Open
bpelakh wants to merge 8 commits into
developfrom
tool-verify
Open

Verify via tool#92
bpelakh wants to merge 8 commits into
developfrom
tool-verify

Conversation

@bpelakh

@bpelakh bpelakh commented Nov 16, 2021

Copy link
Copy Markdown
Contributor

No description provided.

@bpelakh
bpelakh requested a review from pwin November 16, 2021 14:22
@bpelakh
bpelakh marked this pull request as draft November 16, 2021 14:22
@bpelakh

bpelakh commented Nov 16, 2021

Copy link
Copy Markdown
Contributor Author

@pwin This has not been tested at all. It's just an outline of an approach. It needs to be tested, unit tests should be written, and documentation needs to be updated. Feel free to ping me for reviews, etc.

@bpelakh bpelakh left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pwin Other than the small nits I mentioned, the only thing remaining is documentation. Do you want to take a swing at it, perhaps including your working Jena use case as an example? Then we can bump up the version and put this one to rest.

Comment thread onto_tool/bundle_schema.yaml Outdated
required:
- tool
- source
#- shapes

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pwin we should remove this rather than commenting it out.

Comment thread onto_tool/onto_tool.py Outdated
Comment on lines +1092 to +1096
exc_type, exc_value, exc_traceback = sys.exc_info()
traceback.print_tb(exc_traceback, limit=30, file=sys.stdout)
print(exc_value, file=sys.stdout)
print(exc_type, file=sys.stdout)
sys.exit()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need to print the traceback (which should be for internal failures) - if the user provided a bad path for a tool, that should be reported as a regular issue, using logging.error(), we should not have print() statements in the code. Also, a sys.exit(1) should be used to indicate an error termination.

errors = [validation_graph.subjects(RDF.type, sh.ValidationResult)]
assert len(errors) == 1

@pytest.mark.skipif(sys.platform == 'win32', reason="No /bin/cp")

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
@pytest.mark.skipif(sys.platform == 'win32', reason="No /bin/cp")
@pytest.mark.skipif(sys.platform == 'win32', reason="No /bin/cat")

@bpelakh
bpelakh marked this pull request as ready for review November 18, 2021 19:38
@bpelakh bpelakh changed the title Draft: Verify via tool Verify via tool Nov 18, 2021
Comment thread onto_tool/onto_tool.py
rows.append(as_text)
row_format = " ".join(f"{{:{length}.{length}}}" for length in max_length) + "\n"
row_format = "||" + "| ".join(f"{{:{length}.{length}}}" for length in max_length) + "||\n"
rows.sort(key=lambda x: x[0])

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pwin Could this sort be the reason your ORDER BY didn't work for the table?

@pwin

pwin commented Nov 29, 2021 via email

Copy link
Copy Markdown
Contributor

…- it now can return the validation report on stdout
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.

2 participants