Skip to content

Change of behavior in str(pystac.RelType.root) #626

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
TomAugspurger opened this issue Sep 16, 2021 · 4 comments · Fixed by #654
Closed

Change of behavior in str(pystac.RelType.root) #626

TomAugspurger opened this issue Sep 16, 2021 · 4 comments · Fixed by #654

Comments

@TomAugspurger
Copy link
Collaborator

In pystac 1.0, calling str on the value of an enum like RelType.ROOT would return the value:

In [1]: import pystac

In [2]: str(pystac.RelType.ROOT)  # pystac 1.0.0
Out[2]: 'root'

On the latest release (1.1.0) that includes the class name.

In [4]: str(pystac.RelType.ROOT)
Out[4]: 'RelType.ROOT'

I suspect this was changed in #514.

I was relying on the old behavior in https://github.com/TomAugspurger/xstac, which seems useful to me. Do we want to restore the 1.0 behavior? Or stick with the 1.1 behavior?

@lossyrob
Copy link
Member

#514 is a change in API that we didn't catch as such - this should have technically forced a PySTAC 2.0 for semver.

I'd be in favor of reverting back to the 1.0 behavior and releasing as a bugfix version of 1.1.

@duckontheweb @l0b0 thoughts?

@l0b0
Copy link
Contributor

l0b0 commented Sep 16, 2021

@TomAugspurger You can get the same value by using pystac.RelType.ROOT.value; would that work for you?

@lossyrob Whether this should be considered an API change is interesting. The original implementation had a __str__ implementation which was kind of pointless since the same value could be retrieved by using .value. Effectively there were two ways to get the literal used in STAC files, and one of those ways have been removed. Also, underscore methods are considered "private" in Python, and should not be relied on to be stable. It follows that a change (or removal) of an underscore method is not an API change, as far as I'm concerned.

I would be in favour of keeping the current behaviour but informing the users of the removal of the redundant accessor.

@TomAugspurger
Copy link
Collaborator Author

TomAugspurger commented Sep 16, 2021

Yeah, I'm using .value now.

I'm no fan of semver, but this is definitely an API breaking change :)

underscore methods are considered "private" in Python, and should not be relied on to be stable.

There's a difference between private methods (single underscore) and "magic" or dunder methods that are part of the Python data model. Magic methods are meant to be used by top-level functions like str. So by using just public APIs str(pystac.RelType.ROOT) there's a change in the output.

If you really want to remove __str__ as a way to get the string representation from the enum in a backwards compatible way, you can deprecate it by emitting a warning from __str__. Personally, I find str(thing) more natural than thing.value but either works.

@duckontheweb
Copy link
Contributor

This seems related to #652, which is also causing breaking changes in the serialization. I'm in favor of reinstating the old behavior by overwriting the __str__ method.

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.

4 participants