Skip to content

DatacubeExtension.apply is unaware of variables #781

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

Closed
itcarroll opened this issue Mar 31, 2022 · 2 comments · Fixed by #782
Closed

DatacubeExtension.apply is unaware of variables #781

itcarroll opened this issue Mar 31, 2022 · 2 comments · Fixed by #782

Comments

@itcarroll
Copy link
Contributor

The recent addition (#699) of the variable setter did not add an accompanying change to the DatacubeExtension.apply method. As a result, you cannot set dimensions and variables in one swoop with the apply method. The documentation implies that apply should allow you to pass in extension properties.

Additionally, there is no test for the apply method, so adding one seems appropriate now and will help with #324.

@itcarroll
Copy link
Contributor Author

Why does the DatacubeExtension.variables getter return a new dictionary, rather than the result of self._get_property? This quietly breaks ext_item.variables.pop, which will return a name but not remove the variable from ext_item. Feature or bug?

Trying to understand so I can re-write the test_apply_variables method I've introduced in #782 if necessary (or fix a bug).

@duckontheweb
Copy link
Contributor

The result of self._get_property is always going to be a "native" Python type, but the return type of the getter methods often include PySTAC-specific classes. In this case, self._get_property is essentially returning a type of Dict[str, Dict[str, Any]], but the getter needs to return a type of Dict[str, Variable], so we use dictionary comprehension to convert the Dict[str, Any] to a Variable.

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