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

Robokop v5.0 #92

Merged
merged 6 commits into from
Nov 29, 2023
Merged

Robokop v5.0 #92

merged 6 commits into from
Nov 29, 2023

Conversation

YaphetKG
Copy link

@YaphetKG YaphetKG commented Nov 9, 2023

  • New Robokop Graph
  • CDE link fix

@YaphetKG YaphetKG requested a review from mbacon-renci November 9, 2023 13:47
Copy link
Collaborator

Choose a reason for hiding this comment

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

All this looks fine, but I'm going to have to bring this over into the DugPipeline class at some point. What does the addition of line 391 do here? I just want to make sure I'm clear.

Copy link
Author

Choose a reason for hiding this comment

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

total_items is one of the keys that is returned in the response. it looks something like

{ "total_items": 100 
  "dbgap" : {"elements" : [...]}
  "cde" : {"elements" : [...]}
  }

When doing the reduce on line 393 this causes problems since we are trying to gather element IDs and the total items key is an outlier

Copy link

@HowardLander HowardLander left a comment

Choose a reason for hiding this comment

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

Looks fine to me!

@HowardLander HowardLander merged commit 1413e92 into develop Nov 29, 2023
2 checks passed
@HowardLander HowardLander deleted the robokop-v5.0 branch November 29, 2023 20:14
@YaphetKG YaphetKG restored the robokop-v5.0 branch November 29, 2023 20:21
@YaphetKG YaphetKG deleted the robokop-v5.0 branch October 8, 2024 18:27
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.

3 participants