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.

Sign up for GitHub

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 no lineages error #609

Merged
merged 3 commits into from
Jan 11, 2024
Merged

fix no lineages error #609

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ec4fed0
fix no lineages error
Tianhao-Gu Jan 10, 2024
0cec4d3
return no matches
Tianhao-Gu Jan 10, 2024
1a5e60a
move changes to perform_gtdb_lineage_match
Tianhao-Gu Jan 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/service/data_products/genome_attributes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -653,6 +653,13 @@
# later
# could also probably DRY up this and the above method
# retries?
if not lineages:

Check warning on line 656 in src/service/data_products/genome_attributes.py

View check run for this annotation

Codecov / codecov/patch

src/service/data_products/genome_attributes.py#L656 

Added line #L656 was not covered by tests
# return no matches
await storage.update_match_state(

Check warning on line 658 in src/service/data_products/genome_attributes.py

View check run for this annotation

Codecov / codecov/patch

src/service/data_products/genome_attributes.py#L658 

Added line #L658 was not covered by tests
internal_match_id, models.ProcessState.COMPLETE, now_epoch_millis(), []
)
return

Check warning on line 661 in src/service/data_products/genome_attributes.py

View check run for this annotation

Codecov / codecov/patch

src/service/data_products/genome_attributes.py#L661 

Added line #L661 was not covered by tests
Copy link
Member

Choose a reason for hiding this comment

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

You should do this in perform_gtdb_lineage_match, otherwise you'll miss some cases since the matcher chooses one of the two *strategy functions

Copy link
Member

Choose a reason for hiding this comment

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

also maybe make this a function and use that function wherever the same code is called

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

👍 I moved it to perform_gtdb_lineage_match. It's just calling storage.update_match_state. Making this single function call a function?

Copy link
Member

Choose a reason for hiding this comment

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

Fair enough, I guess you'd only be getting rid of a couple args, and the millis function probably shouldn't be abstracted out anyway since it's not mockable and needs to be changed

Copy link
Member

Choose a reason for hiding this comment

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

I guess it is mockable by monkey patching but I hate that


mtch = names.FLD_MATCHES_SELECTIONS
lin = names.FLD_GENOME_ATTRIBS_GTDB_LINEAGE
aql = f"""
Expand Down
1 change: 1 addition & 0 deletions src/service/matchers/lineage_matcher.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ def generate_match_process(
)
if lin:
lineages.add(str(lin))

return CollectionProcess(
process=_process_match,
data_id=internal_match_id,
Expand Down
Loading