Skip to content

Conversation

@DerekFurstPitt
Copy link
Contributor

…rtain fields for get_provenance, particularly omitting expensive ones like files and metadata

…rtain fields for get_provenance, particularly omitting expensive ones like files and metadata
Copy link
Member

@yuanzhou yuanzhou left a comment

Choose a reason for hiding this comment

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

@DerekFurstPitt It's great that you used the CASE form to construct the returned fields. It worked well during my testing. Two changes I'd like to ask:

  1. Add back hubmap_id field for Activity nodes. I noticed a rendering issue on portal-ui and reach out to John, he missed this field in his original list.

  2. Constructing the fields programmatically instead of hardcoding them would make the logic more readable and give us a better control if later we'll add more fields. I put this code snippet as an example:

delimiter = ', '
enitty_properties = ['a', 'b', 'c']
cypher_pairs_list = [f'{x}: node.{x}' for x in enitty_properties]
cypher_pairs_str = delimiter.join(cypher_pairs_list) + delimiter

…o that it is less hard coded and the query values are generated from lists to reduce sources of errors and improve readability
@DerekFurstPitt
Copy link
Contributor Author

@yuanzhou requested changes applied

@yuanzhou yuanzhou mentioned this pull request Jan 13, 2025
@yuanzhou yuanzhou merged commit eaf1a14 into dev-integrate Jan 13, 2025
4 checks passed
@yuanzhou yuanzhou deleted the Derek-Furst/update-prov branch January 13, 2025 15: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