Skip to content

Conversation

@clincoln8
Copy link
Contributor

Currently the auto-flattening of unpack_arcs leads to the following scenario:

Two properties are requested for a given entity but data only exists for one of the properties. The REST api will not contain the property with no data in the response, therefore we cannot tell from only the response the difference between one property requested and one property out of multiple properties requested contains data.

An example:

node_resp = dc_client.node.fetch(node_dcids="bio/APOE", expression="<-[encodesGene,variantID]")
node_resp.get_properties()
> {'bio/APOE': [Node(dcid='bio/AB035149.1', name='AB035149.1', provenanceId='dc/base/NCBI_Gene'...), ...] ...}

In the response from get_properties(), we can't tell if it's for encodesGene or variantID.

A long term fix for this would be to store the original requested properties in the NodeResponse object and only do this "autoflattening" when there was one requested property. However, another consideration is that many return types for a single method can cause a lot confusion.

@clincoln8 clincoln8 requested review from dwnoble and jm-rivera April 17, 2025 20:04
Copy link
Contributor

@jm-rivera jm-rivera left a comment

Choose a reason for hiding this comment

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

Thank you @clincoln8! You're absolutely right that return types should be consistent. In retrospect, I should err a bit less towards convenience and a bit more towards consistency.

@jm-rivera
Copy link
Contributor

A long term fix for this would be to store the original requested properties in the NodeResponse object and only do this "autoflattening" when there was one requested property. However, another consideration is that many return types for a single method can cause a lot confusion.

I think we should just abandon auto-flattening entirely. This is more consistent and there may be other ways to make things easier for users downstream that don't involve quietly doing some magic in the background.

Copy link
Contributor

@dwnoble dwnoble left a comment

Choose a reason for hiding this comment

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

Thanks @clincoln8 !

And agreed with @jm-rivera about auto-flattening that there's probably a better approach.

@clincoln8
Copy link
Contributor Author

Thanks both for the reviews!

@clincoln8 clincoln8 merged commit 3559fc9 into datacommonsorg:master Apr 21, 2025
2 checks passed
@clincoln8 clincoln8 deleted the flatten branch April 21, 2025 20:56
chejennifer added a commit that referenced this pull request Apr 28, 2025
Fixes this error in fetch_entity_names:
https://screenshot.googleplex.com/Bm9BzSUnNYB87N9

There was a recent change that removes auto-flattening when unpacking
arcs (#244), but
fetch_entity_names was still assuming that values would be flattened.

Also updated the tests to use mock responses that are formatted the way
the response would actually come back as.
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