Skip to content

Conversation

@meretp
Copy link
Collaborator

@meretp meretp commented May 12, 2023

This PR fixes two minor issues concerning the wrapper methods introduced in #563.

  • If the default value is used in get_value_from_graph the value doesn't need to be a URIRef, BNode or Literal, so we shouldn't log an error.
  • When parsing the attribution_texts in a package we should use the wrapper methods to prevent a warning concerning type mismatches.

# code that follows
value = graph.value(subject=subject, predicate=predicate, object=_object, default=default, any=_any)
if value and not isinstance(value, (URIRef, Literal, BNode)):
if value != default and value and not isinstance(value, (URIRef, Literal, BNode)):
Copy link
Collaborator

Choose a reason for hiding this comment

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

We need to take a deeper look into what happens here. Probably this should be value is not None instead of just value as value can also be a bool?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If files_analyzed is set in the correpsonding rdf document the return value would be Literal('true'), so value is a boolean iff default is set to a boolean. However, I think we should better use value is not None to "be on the safe side".

@meretp meretp force-pushed the fix-wrapper-methods branch from 6d16199 to 020bd6d Compare May 15, 2023 06:27
Copy link
Collaborator

@armintaenzertng armintaenzertng left a comment

Choose a reason for hiding this comment

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

LGTM

@meretp meretp merged commit 6a7e500 into spdx:main May 15, 2023
@meretp meretp deleted the fix-wrapper-methods branch May 15, 2023 06:34
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