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

process additional context and add to final json results #200

Merged
merged 1 commit into from Feb 14, 2019

Conversation

elrayle
Copy link
Contributor

@elrayle elrayle commented Jan 25, 2019

Adds:

  • context=true|false parameter to QA search URL
  • uses ldpath gem to get context property's values based on ldpath in config for the property
  • add context_mapper_service which fetches the context for each result and adds it to the result hash

Remaining Work after this PR:

  • expand values that are URIs to include URI + label (will be in later PR)
  • figure out why some ldpaths do not parse (handles this gracefully, so will be in later PR)
  • ldpath gem loads (and re-loads) a new graph many times over when the data is available in the passed in graph. Need to explore performance improvements for ldpath.

@elrayle elrayle self-assigned this Jan 25, 2019
@elrayle elrayle force-pushed the ld_refactor_process_context branch 5 times, most recently from 3a73809 to d40c624 Compare February 13, 2019 17:38
@elrayle elrayle changed the title [WIP] process additional context and add to final json results process additional context and add to final json results Feb 13, 2019
@@ -189,6 +189,11 @@ def jsonld?
format == 'jsonld'
end

def context?
context = params.fetch(:context, false)
context == 'true' ? true : false
Copy link
Contributor

Choose a reason for hiding this comment

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

@elrayle Do you care about TRUE or True here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep... I'll downcase before comparison. Good catch.

# @param subject_uri [RDF::URI] the subject within the graph for which the values are being extracted
# @return [<Hash<Symbol><Array<Object>>] mapped context values and information with hash of map key = array of object values for predicates identified in predicate_map.
# @example value map for a single result
# {:uri=>[#<RDF::URI:0x3fcff54a829c URI:http://id.loc.gov/authorities/names/n2010043281>],
Copy link
Contributor

Choose a reason for hiding this comment

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

This should start with a [

Copy link
Contributor

Choose a reason for hiding this comment

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

I also thing if I am reading the code correctly the example should really look like below. I do not see id, label, altlabel being added in construct context.

[{"group" => "group label,
   "property" => "property label",
   "values" => ["value 1","value 2"],
  "selectable" => true,
  "drillable" => false}]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is a copy paste error. I'll update the example.


def context?
context = params.fetch(:context, 'false')
context.casecmp('true').zero?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Switched to use casecmp based on rubocop recommendation. Applied the same change to jsonld? for consistency.

And one other small cleanup to check for blank? instead of nil? for subauth, which covers more potential non-existing values for subauth.

@elrayle
Copy link
Contributor Author

elrayle commented Feb 13, 2019

@Cam156 Your requested changes have been made.

@carolyncole carolyncole merged commit 2eb0b9e into master Feb 14, 2019
@carolyncole carolyncole deleted the ld_refactor_process_context branch February 14, 2019 12:46
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.

None yet

2 participants