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

pystac extension not handling well name property #63

Open
huard opened this issue Oct 11, 2024 · 5 comments · May be fixed by #65
Open

pystac extension not handling well name property #63

huard opened this issue Oct 11, 2024 · 5 comments · May be fixed by #65
Labels
bug Something isn't working implementation Something related to a STAC implementation

Comments

@huard
Copy link
Collaborator

huard commented Oct 11, 2024

The CMIP6Extension has a name property:

    @property
    def name(self) -> SchemaName:
        return get_args(SchemaName)[0]

but this is not handled well by pystac.extensions: https://github.com/stac-utils/pystac/blob/ba406cb7991f992ac6f38026aa779adcc6a4f67a/pystac/extensions/base.py#L258

where getattr(cls, "name") returns a property object, not the actual name.

@fmigneault
Copy link
Collaborator

Can you try replacing by the following and see if that works?

    @classmethod
    def name(cls) -> SchemaName:
        return get_args(SchemaName)[0]

@fmigneault fmigneault added bug Something isn't working implementation Something related to a STAC implementation labels Oct 11, 2024
@huard
Copy link
Collaborator Author

huard commented Oct 11, 2024

That would return a <bound method>.

@mishaschwartz
Copy link
Contributor

geattr(cls, 'name') will return the object defined in the given class that is named name. In this case that is a function. We want the return value of the function, not the function itself. Since the body of the name function does not depend on anything specific to the class itself, we can just refactor the code so name isn't a function anymore and is a class variable instead:

class CMIP6Extension:
    name = get_args(SchemaName)[0]
    ...

minimal example:

from typing import get_args, Literal

SchemaName = Literal["cmip6"]

class CMIP6Extension:
    name = get_args(SchemaName)[0]
    ...

getattr(CMIP6Extension, "name") # returns "cmip6"

Note that if the annotation matters for pydantic we may need to annotate the class variable as well:

name: SchemaName = get_args(SchemaName)[0]

@huard
Copy link
Collaborator Author

huard commented Oct 11, 2024

The class where it appears is not a pydantic BaseModel, so a simple class attribute will work.

@fmigneault
Copy link
Collaborator

Could use pystac's own approach: https://github.com/stac-utils/pystac/blob/ba406cb7991f992ac6f38026aa779adcc6a4f67a/pystac/extensions/classification.py#L174-L181

Otherwise, the simple class attribute as mentioned by @mishaschwartz also works.

@huard huard linked a pull request Oct 16, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working implementation Something related to a STAC implementation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants