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

Fix IndexError in add_definitions by checking list length #603

Merged
merged 2 commits into from
Dec 22, 2023

Conversation

connorjcantrell
Copy link
Contributor

Identified and fixed a test failure caused by an IndexError during the graph population process involving the 'seeAlso' definition. Additionally, to resolve flake8 formatting conflicts with certain SPARQL queries and warning messages, I've implemented # noqa comments to selectively bypass these specific checks.


# add definitions, citations to the graph
for definition in dictionary:
term = URIRef(definition[0])
if len(definition) > len(header):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It appears some row's definitions are not wrapped in parentheses and using commas within the definition, which is causing parsing issues. This new condition should catch most of them.

Copy link
Member

Choose a reason for hiding this comment

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

There was one bad row which you can fix in definitions.csv (I think it is the Tablet row)

Copy link
Member

@gtfierro gtfierro left a comment

Choose a reason for hiding this comment

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

Thanks!

@gtfierro gtfierro merged commit 6dcb766 into BrickSchema:master Dec 22, 2023
2 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.

2 participants