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

Value attribute differently named in odml and nixpy #308

Closed
mpsonntag opened this issue Nov 5, 2018 · 4 comments · Fixed by #313
Closed

Value attribute differently named in odml and nixpy #308

mpsonntag opened this issue Nov 5, 2018 · 4 comments · Fixed by #313
Assignees

Comments

@mpsonntag
Copy link
Contributor

The Property.value accessor is differently named in odml (Property.value) and nixpy (Property.values).

Should we unify this to either of the attribute names? This would be a breaking change.

@jgrewe
Copy link
Member

jgrewe commented Nov 6, 2018

I guess this should indeed be named in the same way. I am completely uncertain whether to favour p.value or p.values.
This does not necessarily mean a breaking change, though, we could support both for a while and deprecate the one or the other.

@JuliaSprenger
Copy link
Contributor

I think p.values is more intuitive because it is always a list being returned and never a single item.

@achilleas-k
Copy link
Member

I was about to suggest making it value as it sounds (in my opinion) more general. My thinking was that even when it's multiple values, you can think of it as the value of this property is a list. @JuliaSprenger makes a good point though that even in single value cases, we return a tuple, so values signals that we're returning some sort of iterable or collection.

So I vote for values.

To @jgrewe's point, we could also keep both. Is there a good reason for deprecating one in the long run? I guess it might lead to some confusion. Two properties where one is an alias for the other isn't common so some may take it as a signal that there's a difference (unless they read the documentation). That said, I wouldn't be against having both without deprecating (any time soon).

@mpsonntag
Copy link
Contributor Author

I would vote for p.values in odml and keeping p.value with a deprecation warning for the next year (or other arbitrary timeframe).

@mpsonntag mpsonntag self-assigned this Nov 6, 2018
mpsonntag added a commit to mpsonntag/python-odml that referenced this issue Nov 21, 2018
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