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 for infixowl issues 1453 and 944 #1530

Merged
merged 6 commits into from Jan 2, 2022
Merged

Fix for infixowl issues 1453 and 944 #1530

merged 6 commits into from Jan 2, 2022

Conversation

ghost
Copy link

@ghost ghost commented Dec 25, 2021

Fixes #1453 and #944

Proposed Changes

  • Change falsy if i to if i is None to avoid improper Exception when cardinality == 0 (infixowl cardinality - 0 #1453)
  • Change doctest examples to use << >> infix delimiters to avoid | clashing with SPARQL paths (Only URIRefs or Paths can be in paths! #944)
  • Translate/transcribe doctests as pytests in separate file as a foundation for developing some unit tests
  • unskip some (passing) doctests

Tranlate/transcribe doctests as pytests in separate file as a foundation for developing some unit tests, unskip doctests
@ghost ghost mentioned this pull request Dec 25, 2021
test/test_infixowl.py Outdated Show resolved Hide resolved
test/test_infixowl.py Outdated Show resolved Hide resolved
test/test_infixowl.py Outdated Show resolved Hide resolved
@nicholascar nicholascar self-requested a review December 28, 2021 11:19
Copy link
Member

@nicholascar nicholascar left a comment

Choose a reason for hiding this comment

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

Approving but not merging in case @gjhiggins wants to act on @aucampia's suggestions in comments

@ghost
Copy link
Author

ghost commented Dec 28, 2021

Approving but not merging in case @gjhiggins wants to act on @aucampia's suggestions in comments

How diplomatic of you both. I got derailed, of course I meant to use str ;)

@aucampia
Copy link
Member

aucampia commented Dec 28, 2021

I will do a re-check tomorrow night.

@aucampia aucampia self-requested a review December 28, 2021 23:16
Copy link
Member

@aucampia aucampia left a comment

Choose a reason for hiding this comment

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

Change looks good, just want someone else to explicitly confirm the addition of __matmul__ and __rmatmul__.

@nicholascar
Copy link
Member

Merging in with __matmul__ and __rmatmul__ (use of @) adnd will have to review if/when other Manchester syntax implementations, like @hsolbrig's appear.

@nicholascar nicholascar merged commit f7c6146 into RDFLib:master Jan 2, 2022
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.

infixowl cardinality - 0
2 participants