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 utils.parse_object() #46

Merged
merged 10 commits into from
Dec 13, 2022
Merged

Fix utils.parse_object() #46

merged 10 commits into from
Dec 13, 2022

Conversation

jesper-friis
Copy link
Contributor

@jesper-friis jesper-friis commented Dec 3, 2022

Closes #45

General cleanup

  • Fixing failing tests by improved generality of parse_literal() and parse_object().
  • Added __eq__() and __hash__() methods to Literal. The __eq__() method is required for proper testing...
  • Reduced code duplication.
  • Hopefully also addressed the comments by @quaat in PR Added collection backend #44

…ing tests.

Reduced code dublication.
Hopefully addressed comments by Thomas in PR #44
@jesper-friis jesper-friis linked an issue Dec 3, 2022 that may be closed by this pull request
@codecov-commenter
Copy link

codecov-commenter commented Dec 3, 2022

Codecov Report

Base: 74.25% // Head: 74.69% // Increases project coverage by +0.43% 🎉

Coverage data is based on head (8c5985c) compared to base (3cc3199).
Patch coverage: 73.77% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #46      +/-   ##
==========================================
+ Coverage   74.25%   74.69%   +0.43%     
==========================================
  Files          12       12              
  Lines         703      727      +24     
==========================================
+ Hits          522      543      +21     
- Misses        181      184       +3     
Impacted Files Coverage Δ
tripper/namespace.py 92.77% <50.00%> (+1.63%) ⬆️
tripper/utils.py 84.00% <69.56%> (-2.05%) ⬇️
tripper/literal.py 84.84% <100.00%> (+1.51%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

quaat
quaat previously approved these changes Dec 3, 2022
@@ -20,6 +20,47 @@
assert infer_iri(coll.meta) == coll.meta.uri
assert infer_iri(coll) == coll.uuid

# We have no dependencies on pydantic, hence don't assume that it is installed
Copy link
Contributor

Choose a reason for hiding this comment

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

A good starting point could be to use (or expand on) this datamodel: https://github.com/SINTEF/soft7/blob/main/s7/pydantic_models/soft7.py

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I renamed uri to identity in the test. With that it seems that we are pretty much matching the soft7 datamodel.

However, it is confusing and not very useful to have two different names for the same thing in soft7 and dlite. I have nothing in principle against updating dlite, but it would take several hours to change uri to identity allover dlite when also keeping backward compatibility. It is time I don't have. Great if someone would like to do it, otherwise I would suggest to update soft7 instead.

try:
from pydantic import AnyUrl, BaseModel, Field
except ImportError:
pass
Copy link
Contributor

Choose a reason for hiding this comment

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

Try to avoid except pass - they are notoriously difficult to reason about

)
# Format not supported in Python < 3.11
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove commented-out code

@@ -112,7 +158,5 @@ def h():
assert parse_object(XSD.int) == XSD.int
assert parse_object(f'"42"^^{XSD.integer}') == Literal("42", datatype=XSD.integer)
assert parse_object(f'"4.2"^^{XSD.double}') == Literal("4.2", datatype=XSD.double)

# __FIXME__: parse_object() currently fails for the following cases:
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove commented-out code

@jesper-friis jesper-friis merged commit e8f3aeb into main Dec 13, 2022
@jesper-friis jesper-friis deleted the 45-update-parse_object branch December 13, 2022 13:43
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.

Fix utils.parse_object()
3 participants