Result-class API and module split (v0.3.0) - #46
Open
natalie-23-gill wants to merge 16 commits into
Open
Conversation
…raction Integrate 52 upstream commits (pathfinder/neighborhood/kg_loader/graph_downloader pipelines, metakg URL fallback, Plover error handling, trapi return_json=False) with the branch's refactor (results/visualization/attribute_extraction modules, multi-hop TRAPI builder, backward-compat adapters). Key conflict resolutions: - trapi.py: adopt upstream return_json=False default and query() type guard; keep branch HopSpec/build_multi_hop_query and timeout=60. - translator_query.py: keep branch TranslatorResources/KnowledgeGraph structure plus upstream format_query_json/build_attribute_constraint (auto-merged). - translator_metakg.py: combine branch PLOVER_APIS loop with upstream per-request try/except resilience in _add_plover_api_entry; take upstream use_new_url find_link/get_KP_metadata. - TCT.py: rename Neiborhood_finder to Neighborhood_finder (upstream spelling) with the branch resources= signature and NeighborhoodResult return; keep deprecated Neiborhood_finder alias. Drop upstream hunks for visualization functions the branch relocated. - visualization.py: port upstream empty-data guards into plot_heatmap and visulization_one_hop_ranking, and output_filename_prefix into visualize_neighborhood_graph. - Notebooks: prefer upstream for all conflicting notebooks. - pyproject.toml/requirements.txt: union deps (igraph, zstandard, scipy); uv.lock regenerated.
Cover TCT_pathfinder, TCT_neighborhood_finder, kg_loader, and graph_downloader (the four modules upstream added without tests) so the 95% coverage gate holds. The neighborhood/pathfinder parse tests also lock in compatibility with the branch's KnowledgeGraph wrapper returned by parallel_api_query. Fix a latent bug in kg_loader.load_kg2/load_kg2_networkx/load_kg2_igraph: they passed edges_to_include/remove_unused_nodes positionally, so remove_unused_nodes (a bool) landed in the edges_to_include slot and crashed with "argument of type 'bool' is not iterable". Pass them by keyword.
Fold the branch's result-class API into the notebooks that the upstream merge took from upstream. Resource loading now uses TranslatorResources.load() (with legacy APInames/metaKG aliases so exploration cells keep working), and the finders use Neighborhood_finder/Path_finder returning NeighborhoodResult/ PathResult plus parallel_api_query(resources=...). - Neighborhood_finder, Path_finder, Connection_finder, Network_finder: result-class finders. - overview_of_KGs, Connecting_userAPI: TranslatorResources loading; Connecting_userAPI builds a TranslatorResources from the mutated structures so the custom AMLKG registration is honored. - Annotate_graph: replace the stale CSV metaKG + old parallel_api_query signature with TranslatorResources + the current signature. - Test_neighborhood_vis and name_resolver_lookup already used the modern API (unchanged). Outputs on modified cells are cleared; downstream outputs may be stale until a re-run against the live Translator APIs. Expand the CHANGELOG 'Merged from upstream' section with concise bullets for the new module APIs, name_resolver batch lookup, node_normalizer guards, metakg URL/ limit change, visualization additions, and the new upstream notebooks.
Sync with NCATSTranslator/main (42 commits). Resolutions:
- TCT.py, translator_query.py: kept result-class API and ChatGPT
functions; folded in upstream's URL strip('/') fix and dead-code
removal (parse_result_old and the deprecated ranking helpers).
- Accepted upstream's deletion of legacy notebooks; kept our
result-class modernized notebooks for the shared ones.
- Aligned with upstream by removing the deprecated backward-compat
ranking functions and their orphaned tests.
- Fixed a None-crash in upstream's format_query_json_for_pathfinder_
with_constraints (bare if -> elif after the None guard).
Upstream switched query_aragorn_pathfinder from shepherd.renci.org to shepherd.ci.transltr.io; the endpoint tests asserted the old host.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR introduces a result-class API for TCT's knowledge-graph finders, splits
TCT.pyinto focused modules, adds multi-hop TRAPI queries and edge-attribute extraction. Bumps version to 0.3.0. These are features ported over from GeneSet_Translator and are aimed at making building on tools on top of TCT easier.Neighborhood_finder()andPath_finder()now return typed result objects instead of long tuples:Existing tuple-unpacking still works (with a
DeprecationWarning), so this is backward compatible.What's in this PR
New: result API (
results.py)KnowledgeGraph,ParsedKnowledgeGraph,NeighborhoodResult,PathResult, anddataframe_to_graph()..to_networkx()on every result type.resolve_names=Trueaddslabeland biolinkcategoriesto nodes;include_attributes=Trueadds publications, supporting text, and confidence scores to edges..to_dataframe(),.parse(),.rank(), and dict-like access on the graph wrappers.New: query and attribute helpers
attribute_extraction.py- publications, supporting text, and confidence scores from TRAPI edge attributes, including nested study results.HopSpec+build_multi_hop_query()intrapi.pyfor chained multi-hop queries.Refactor
TCT.pyreduced from 2,314 to 1,492 lines, with plotting extracted tovisualization.py.translator_resources.py-TranslatorResourcesreplaces the(APInames, metaKG, API_predicates)triplet. Legacy kwargs still work with a deprecation warning.verboseflag, a clearValueErroron unresolvable input nodes, andTranslatorNode.nameas an alias for.label.Upstream sync
main(43 commits), with tests added for the new upstream modules to keep the coverage gate passing.TypeErrorinformat_query_json_for_pathfinder_with_constraints()whenconstraints=None(a bareifafter theNoneguard should have beenelif).Docs
CHANGELOG.md, in Keep a Changelog format. Documents this release and back-fills the eight prior ones from 0.1.0 onward, reconstructed from git history.consuming_resultspage documenting the result contract: result fields,to_networkx()node/edge attributes, andrankedcolumns.sphinx-apidocand wired intodocs/Makefile, covering eight modules that previously had no page, includingresults.py.Suggested review order
64 files, but weighted toward tests (+5,940 lines) and notebooks (+2,085); library source is +1,912.
TCT/results.py- the core of the PR; everything follows from this contract.docs/source/consuming_results.md- the same contract in prose.TCT/TCT.py- largest diff, but mostly code moving out to the new modules.TCT/attribute_extraction.py,translator_resources.py,trapi.py- small and self-contained.TCT/visualization.py- near-verbatim move.Backward compatibility
Tuple-unpacking, legacy
APInames/metaKG/API_predicateskwargs, and theNeiborhood_finderalias all still work with deprecation warnings.parse_KG()andrank_by_primary_infores()accept both raw dicts and result objects.Three intentional behavior changes:
verbose=Trueto restore.ValueErrorinstead of a bareAttributeError.extract_publications()normalizes bare PubMed ids (12345) toPMID:12345.Testing
Full suite run against live Translator APIs: 394 passed, 1 skipped, 4 failed. Docs build verified with
make html. No new ruff findings.The 4 failures are all in
test_nodenorm.pyand reproduce on currentmain, so they are not caused by this PR. Both come from Node Normalizer production drift:/statusno longer returnsbabel_version, andUNII:8HB6JQ9DFTnow resolves tobotulinum toxin type arather than the expectedDysport. Happy to fix these here, though the corrected expectations are arguably yours to choose.Notes for maintainers
v0.3.0on merge (the publishedv0.2.0predatesresults.py, so no released version carries this API).Neighborhood_finder/Path_finderand the upstreamTCT_neighborhood_finder/TCT_pathfindermodules currently coexist. Unifying them is left as follow-up to keep this PR reviewable.