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

should hydrate coerce None to an empty dictionary or list? #7

Closed
mishaschwartz opened this issue Nov 19, 2024 · 1 comment · Fixed by #8
Closed

should hydrate coerce None to an empty dictionary or list? #7

mishaschwartz opened this issue Nov 19, 2024 · 1 comment · Fixed by #8

Comments

@mishaschwartz
Copy link

The tests for pypgstac make it seem like the hydrate function is expected to be able to treat a None value as if it were an empty dictionary:

    def test_base_none(self) -> None:
        base_item = {"value": None}
        dehydrated = {"value": {"a": "b"}}
        hydrated = self.hydrate(base_item, dehydrated)
        assert hydrated == {"value": {"a": "b"}}

See the relevant code here: `https://github.com/stac-utils/pgstac/blob/6c569c0ee56bd9519ad947cbcaa8bfbcfade15bb/src/pypgstac/tests/hydration/test_hydrate.py#L241-L245

Right now, the hydrate function does not do that. Instead it tries to downcast the None value to a dict and raises an error since it can't do that:

hydraters/src/lib.rs

Lines 42 to 49 in d3946d3

if let Ok(item) = item.downcast::<PyDict>() {
if let Ok(base) = base.downcast::<PyDict>() {
hydrate_dict(base, item)?;
} else {
return Err(PyValueError::new_err(
"type mismatch: item is a dict, but the base was not",
));
}

Do we want to change the code here so that if item or base is None, we convert it to a dict or list? Or if not, we should update the tests in pgstac.

What do you think?

@gadomski
Copy link
Collaborator

Yup, I'll update — I must have pulled the test suite from somewhere else so I didn't get stac-utils/pgstac#263. Thanks for the catch.

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 a pull request may close this issue.

2 participants