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.

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

Support collection.abc.Mapping as resource instead of only dict #44

Merged
merged 4 commits into from
Jan 13, 2025

Conversation

axelv
Copy link
Contributor

@axelv axelv commented Jan 3, 2025

This slight modification to the engine relaxes the assumption that a resource node must be a Python dictionary by changing
some isinstance checks:

- if isinstance(node.data, dict):
+ if isinstance(node.data, Mapping):

This allows us to pass Pydantic objects or dataclasses and preserve metadata and methods defined on those objects and solves the issue described here #43.

Let me know what you think and if adaptation are needed 🙌

FYI: I think a similar relaxation of list to Sequence would also be possible 🙂

@axelv
Copy link
Contributor Author

axelv commented Jan 6, 2025

@ruscoder What do you think of this approach?

@ir4y ir4y requested a review from ruscoder January 12, 2025 22:05
@ruscoder
Copy link
Member

Hi @axelv, it looks good to me, thanks.

@ruscoder ruscoder merged commit 94a55db into beda-software:master Jan 13, 2025
@axelv
Copy link
Contributor Author

axelv commented Jan 13, 2025

FYI: I think a similar relaxation of list to Sequence would also be possible 🙂

@ruscoder what do you think of this? Is it OK if I start a PR for this?

@ruscoder
Copy link
Member

@axelv it will be great to have it here

@axelv
Copy link
Contributor Author

axelv commented Jan 19, 2025

FYI: I think a similar relaxation of list to Sequence would also be possible 🙂

This is harder than I thought because str is also a Sequence 🤔

@ruscoder
Copy link
Member

@axelv consider using MutableSequence

In [4]: isinstance("ok", MutableSequence)
Out[4]: False

In [5]: isinstance((1,2,3), MutableSequence)
Out[5]: False

In [6]: isinstance([1,2,3], MutableSequence)
Out[6]: True

@ruscoder
Copy link
Member

ruscoder commented Jan 19, 2025

@axelv btw, do you have a real-world use case for using list-like structures together with fhirpath py?

@axelv
Copy link
Contributor Author

axelv commented Jan 19, 2025

Yes, I want create custom lists to make slices accessible by name in Python: https://github.com/axelv/pydantic-fhir-slicing
But still work in progress

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.

2 participants