RE2022-275: refactor filtering handler

[Tianhao-Gu]

No description provided.

[codecov]

Codecov Report

Attention: 48 lines in your changes are missing coverage. Please review.

Comparison is base (e37d29c) 47.08% compared to head (e912e22) 47.11%.

Files	Patch %	Lines
src/service/filtering/filtering_processing.py	21.56%	40 Missing ⚠️
src/common/product_models/heatmap_common_models.py	50.00%	5 Missing ⚠️
src/service/data_products/genome_attributes.py	25.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #584      +/-   ##
==========================================
+ Coverage   47.08%   47.11%   +0.03%     
==========================================
  Files          74       75       +1     
  Lines        6407     6422      +15     
==========================================
+ Hits         3017     3026       +9     
- Misses       3390     3396       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[MrCreosote]

I would structure things a little differently, assuming I'm interpreting what you plan to do correctly. I think you can move all the filtering code to filters.py and abstract out the lines that get the columns:

    column_meta = await _get_genome_attributes_meta_internal(
        appstate.arangostorage, coll_id, load_ver, load_ver_override)

You'd abstract that into a function / lambda for genome attribs and pass it to the filtering code, then when doing heatmaps you'd make a different function that pulls the heatmap meta and translates it to attribs columns.

I still think that it'd be cleaner to make the filters understand heatmap column types directly but that seems like a lot more work than the translation mechanism...

[MrCreosote]

I'm also wondering if we should put the filter processing code in a new file like filter_processing.py or something

[Tianhao-Gu]

I would structure things a little differently, assuming I'm interpreting what you plan to do correctly. I think you can move all the filtering code to filters.py and abstract out the lines that get the columns:

    column_meta = await _get_genome_attributes_meta_internal(
        appstate.arangostorage, coll_id, load_ver, load_ver_override)

You'd abstract that into a function / lambda for genome attribs and pass it to the filtering code, then when doing heatmaps you'd make a different function that pulls the heatmap meta and translates it to attribs columns.

I still think that it'd be cleaner to make the filters understand heatmap column types directly but that seems like a lot more work than the translation mechanism...

👍 abstract filter handler to filter_processing.py

[Tianhao-Gu]

test local endpoint.

(dev) tgu@collections (dev_heatmap_filtering)$curl -X 'GET' \ 'http://127.0.0.1:5000/collections/PMI/data_products/genome_attribs/?sort_on=kbase_id&sort_desc=false&skip=0&limit=1&output_table=false&count=false&conjunction=false&match_mark=false&selection_mark=false&filter_Completeness=%5B80,90%5D' \ -H 'accept: application/json' \ -H "Authorization: Bearer $token" | python -m json.tool % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 100 7938 100 7938 0 0 23672 0 --:--:-- --:--:-- --:--:-- 23837 { "skip": 0, "limit": 1, "fields": null, "table": null, "data": [ { "Completeness": 85.21, "Contamination": 0, "classification": "d__Bacteria;p__Actinomycetota;c__Actinomycetia;o__Streptomycetales;f__Streptomycetaceae;g__Streptomyces;s__Streptomyces sp004217285", "classification_method": "taxonomic classification defined by topology and ANI", "closest_placement_af": 1, "closest_placement_ani": 100, "closest_placement_radius": 95, "closest_placement_reference": "GCF_004217285.1", "closest_placement_taxonomy": "d__Bacteria;p__Actinomycetota;c__Actinomycetia;o__Streptomycetales;f__Streptomycetaceae;g__Streptomyces;s__Streptomyces sp004217285", "fastani_af": 1, "fastani_ani": 100, "fastani_reference": "GCF_004217285.1", "fastani_reference_radius": 95, "fastani_taxonomy": "d__Bacteria;p__Actinomycetota;c__Actinomycetia;o__Streptomycetales;f__Streptomycetaceae;g__Streptomyces;s__Streptomyces sp004217285", "kbase_display_name": "Streptomyces_sp._BK239_2799112261.fna", "kbase_id": "69278_956_1", "kbase_sample_id": null, "msa_percent": 95.01, "note": "topological placement and ANI have congruent species assignments", "other_related_references(genome_id,species_name,radius,ANI,AF)": "GCF_014649375.1, s__Streptomyces aurantiogriseus, 95.0, 88.16, 0.811; GCA_021568835.1, s__Streptomyces sp021568835, 95.0, 87.3, 0.759; GCF_001896135.2, s__Streptomyces phaeoluteigriseus, 95.0, 87.28, 0.772; GCF_002154575.1, s__Streptomyces bobili, 95.0, 87.16, 0.642; GCF_900129855.1, s__Streptomyces sp900129855, 95.0, 87.06, 0.778; GCF_014648915.1, s__Streptomyces phaeofaciens, 95.0, 86.93, 0.787; GCF_003112535.1, s__Streptomyces rishiriensis, 95.0, 86.85, 0.778; GCF_013085465.1, s__Streptomyces cacaoi_A, 95.0, 86.84, 0.815; GCA_001298545.1, s__Streptomyces sp001298545, 95.0, 86.82, 0.799; GCF_001746365.1, s__Streptomyces sp001746365, 95.0, 86.82, 0.765; GCF_900110755.1, s__Streptomyces sp900110755, 95.0, 86.8, 0.767; GCF_002019855.1, s__Streptomyces antibioticus_B, 95.0, 86.77, 0.759; GCF_001514205.1, s__Streptomyces griseoruber, 95.0, 86.75, 0.788; GCF_001184025.1, s__Streptomyces sp001184025, 95.0, 86.74, 0.801; GCF_024539675.1, s__Streptomyces sp024539675, 95.0, 86.73, 0.749; GCF_024539815.1, s__Streptomyces sp024539815, 95.0, 86.63, 0.734; GCF_001514285.1, s__Streptomyces sp001514285, 95.0, 86.62, 0.77; GCF_016860545.1, s__Streptomyces asoensis, 95.0, 86.61, 0.793; GCF_001428885.1, s__Streptomyces sp001428885, 95.0, 86.49, 0.788; GCF_014649655.1, s__Streptomyces humidus, 95.0, 86.46, 0.768; GCF_003032475.1, s__Streptomyces sp003032475, 95.0, 86.37, 0.792; GCF_000720255.1, s__Streptomyces griseus_I, 95.0, 86.22, 0.78; GCF_001514265.1, s__Streptomyces resistomycificus, 95.0, 85.77, 0.759; GCF_001279775.1, s__Streptomyces sp001279775, 95.0, 85.73, 0.742; GCF_001513975.1, s__Streptomyces curacoi, 95.0, 85.62, 0.738; GCF_001905845.1, s__Streptomyces sp001905845, 95.0, 85.61, 0.724; GCF_001282115.1, s__Streptomyces sp001282115, 95.0, 85.6, 0.671; GCF_023544665.1, s__Streptomyces sp023544665, 95.0, 85.58, 0.7; GCF_019049245.1, s__Streptomyces sp019049245, 95.0, 85.58, 0.763; GCA_000720805.1, s__Streptomyces violaceoruber_B, 96.8346, 85.56, 0.732; GCF_002291145.1, s__Streptomyces sp002291145, 95.0, 85.56, 0.722; GCF_014596915.1, s__Streptomyces sp014596915, 95.0, 85.55, 0.745; GCF_003933965.1, s__Streptomyces sp003933965, 95.0, 85.54, 0.625; GCF_022513565.1, s__Streptomyces sp022513565, 95.0, 85.5, 0.74; GCF_001514065.1, s__Streptomyces antibioticus, 95.2614, 85.5, 0.759; GCF_021474425.1, s__Streptomyces muensis, 95.0, 85.5, 0.68; GCF_023516615.1, s__Streptomyces panaciradicis, 95.0, 85.5, 0.724; GCF_021173675.1, s__Streptomyces lincolnensis_A, 95.0, 85.49, 0.773; GCF_015712165.1, s__Streptomyces spinoverrucosus_A, 95.0, 85.48, 0.725; GCF_006715395.1, s__Streptomyces sp006715395, 95.0, 85.47, 0.754; GCF_000721105.1, s__Streptomyces cellulosae_A, 96.8346, 85.45, 0.749; GCF_018070565.1, s__Streptomyces sp018070565, 95.0, 85.45, 0.682; GCF_900091315.1, s__Streptomyces sp900091315, 95.0, 85.45, 0.699; GCF_008806595.1, s__Streptomyces luteolifulvus, 95.0, 85.44, 0.714; GCF_019219825.1, s__Streptomyces sp019219825, 95.0, 85.42, 0.721; GCF_001514195.1, s__Streptomyces griseorubiginosus, 95.0, 85.42, 0.76; GCF_004028635.1, s__Streptomyces cyaneus, 95.0, 85.41, 0.759; GCF_008973465.1, s__Streptomyces sp008973465, 95.0, 85.41, 0.69; GCF_009377175.1, s__Streptomyces adustus, 95.0, 85.39, 0.689; GCF_013044505.1, s__Streptomyces sp013044505, 95.0, 85.39, 0.708; GCF_001509795.1, s__Streptomyces sp001509795, 95.0, 85.38, 0.705; GCF_003443735.1, s__Streptomyces sviceus, 95.0589, 85.38, 0.774; GCF_001514235.1, s__Streptomyces caeruleatus, 95.0, 85.37, 0.755; GCF_004187715.1, s__Streptomyces sp004187715, 95.0, 85.35, 0.716; GCF_000719285.1, s__Streptomyces bicolor, 95.0, 85.35, 0.735; GCF_003851625.1, s__Streptomyces sp003851625, 95.0, 85.35, 0.76; GCF_017614965.1, s__Streptomyces griseorubiginosus_B, 95.0, 85.34, 0.664; GCF_000718165.1, s__Streptomyces fulvoviolaceus, 95.0, 85.34, 0.765; GCF_021538895.1, s__Streptomyces olivochromogenes_A, 95.0, 85.33, 0.708; GCF_001514305.1, s__Streptomyces dysideae, 95.0, 85.33, 0.743; GCF_003074055.1, s__Streptomyces nigra, 95.0, 85.33, 0.712; GCF_022698165.1, s__Streptomyces sp022698165, 95.0, 85.31, 0.75; GCF_006539505.1, s__Streptomyces spinoverrucosus, 95.0, 85.31, 0.709; GCF_018128905.1, s__Streptomyces sp003814525, 95.0, 85.31, 0.769; GCF_010548375.1, s__Streptomyces sp010548375, 95.0, 85.3, 0.749; GCF_003963535.1, s__Streptomyces cyaneochromogenes, 95.0, 85.29, 0.754; GCF_000720765.1, s__Streptomyces sp000720765, 95.0, 85.28, 0.758; GCF_019049285.1, s__Streptomyces sp019049285, 95.0, 85.26, 0.73; GCF_014257025.1, s__Streptomyces sp014257025, 95.0, 85.22, 0.725; GCF_015160875.1, s__Streptomyces chromofuscus, 95.0, 85.2, 0.705; GCF_022698145.1, s__Streptomyces sp000411315, 95.0, 85.19, 0.735; GCF_001509475.1, s__Streptomyces regalis, 95.0, 85.18, 0.729; GCF_000349325.1, s__Streptomyces davaonensis, 95.0, 85.16, 0.733; GCF_021261325.1, s__Streptomyces sp021261325, 95.0, 85.16, 0.759; GCF_019857225.1, s__Streptomyces plumbidurans, 95.0, 85.14, 0.686; GCA_000715615.1, s__Streptomyces cinnabarinus, 95.0, 85.11, 0.735; GCF_001270495.1, s__Streptomyces viridochromogenes_A, 95.0, 85.11, 0.749; GCF_900103585.1, s__Streptomyces sp900103585, 95.0, 85.1, 0.756; GCF_017948455.1, s__Streptomyces sp017948455, 95.0, 85.09, 0.728; GCF_014655715.1, s__Streptomyces lanatus, 95.0, 85.01, 0.755; GCF_009709555.1, s__Streptomyces blattellae, 95.0, 84.99, 0.693; GCF_000974985.2, s__Streptomyces mangrovisoli, 95.0, 84.97, 0.699; GCF_005222485.1, s__Streptomyces griseoviridis_A, 95.0, 84.94, 0.729; GCF_002217755.1, s__Streptomyces hyaluromycini, 95.0, 84.93, 0.744; GCF_001005085.2, s__Streptomyces humi, 95.0, 84.92, 0.73; GCF_900105755.1, s__Streptomyces sp900105755, 95.0, 84.91, 0.739; GCF_017948485.1, s__Streptomyces sp017948485, 95.0, 84.9, 0.737; GCF_003814875.1, s__Streptomyces sp003814875, 95.0, 84.9, 0.753; GCF_021462265.1, s__Streptomyces shenzhenensis, 95.0, 84.82, 0.735; GCA_019510245.1, s__Streptomyces sp019510245, 95.0, 84.81, 0.704; GCF_900091845.1, s__Streptomyces sp900091845, 95.0, 84.8, 0.686; GCF_014645815.1, s__Streptomyces fuscichromogenes, 95.0, 84.77, 0.749; GCA_013362785.1, s__Streptomyces sp013362785, 95.0, 84.5, 0.544; GCF_023516595.1, s__Streptomyces neyagawaensis, 95.4948, 84.36, 0.733; GCF_008386495.1, s__Streptomyces tailanensis, 95.0, 84.12, 0.706; GCF_002214185.1, s__Streptomyces capitiformicae, 95.0, 84.04, 0.718; GCF_014489635.1, s__Streptomyces roseirectus, 95.0, 84.01, 0.735; GCF_001419795.1, s__Streptomyces niveiscabiei, 95.0, 83.94, 0.556; GCF_019104725.1, s__Streptomyces corallincola, 95.0, 83.84, 0.593", "pplacer_taxonomy": "d__Bacteria;p__Actinomycetota;c__Actinomycetia;o__Streptomycetales;f__Streptomycetaceae;g__Streptomyces;s__", "red_value": null, "translation_table": 11, "user_genome": "69278_956_1_kbase", "warnings": null } ], "count": null }

[MrCreosote]

Kind of a big refactor, maybe, so I'm leaving things here

[Tianhao-Gu]

Kind of a big refactor, maybe, so I'm leaving things here

Most of the codes are exact same as existing codes. Just need to abstract them to a new location so that heatmap can use them.

[MrCreosote]

Kind of a big refactor, maybe, so I'm leaving things here

Most of the codes are exact same as existing codes. Just need to abstract them to a new location so that heatmap can use them.

I was talking about the comment discussing getting rid of the column meta function and just passing it in directly

[MrCreosote]

LGTM other than one type hint