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 duplicate curies issue #432

Merged
merged 10 commits into from
Sep 11, 2023
Merged

Fix duplicate curies issue #432

merged 10 commits into from
Sep 11, 2023

Conversation

maximusunc
Copy link
Collaborator

No description provided.

Copy link
Collaborator

@kennethmorton kennethmorton left a comment

Choose a reason for hiding this comment

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

Small changes. Take em or leave em.

@@ -496,6 +501,9 @@ async def lookup(
# add new result to hashmap
output_results[sub_result_hash] = result

stop_merging = datetime.datetime.now()
message_merging_time += (stop_merging - start_merging).total_seconds()
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should really be using time.time for timing such as these. Not a big deal but FYI

@@ -164,6 +164,9 @@ async def generate_from_kp(
)
for batch_results in batch(onehop_results, 1_000_000):
result_map = defaultdict(list)
# copy subqgraph between each batch
# before we fill it with result curies
populated_subqgraph = copy.deepcopy(subqgraph)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think another comment or two describing why we need to do this would be helpful. Just describe the problem we are solving right now.

@maximusunc maximusunc merged commit f80c899 into master Sep 11, 2023
4 checks passed
@maximusunc maximusunc deleted the fix_duplicate_curies branch September 11, 2023 18:29
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