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

revision num print out by __repr__ #40

Open
ke-zhang-rd opened this issue Aug 12, 2019 · 3 comments
Open

revision num print out by __repr__ #40

ke-zhang-rd opened this issue Aug 12, 2019 · 3 comments

Comments

@ke-zhang-rd
Copy link

https://github.com/danielballan/amostra/blob/first-pass/amostra/objects.py#L44

I feel user may want to see revision num when then dolist(sample.revisions())

@ke-zhang-rd ke-zhang-rd changed the title revision num print out in __repr__ revision num print out by __repr__ Aug 12, 2019
@danielballan
Copy link
Contributor

danielballan commented Aug 12, 2019

I think this is worth considering. But let me explain my reasoning for leaving it out so we can weigh our options. Many Python objects have __repr__s that, if you type them out, create an identical object. Examples include lists, dicts, and Counters:

>>> a = [1, 2, 3]
>>> a
[1, 2, 3]
>>> b = {'thing': 'stuff'}
>>> b
{'thing': 'stuff'}
>>> import collections
>>> c = collections.Counter({'a': 1, 'b': 2, 'c': 2})
>>> c
Counter({'b': 2, 'c': 2, 'a': 1})

This has called an "eval-able repr" because eval(repr(obj)) == obj. Obeying this pattern is not a requirement. There are plenty of objects in Python that don't follow it, but it's nice to abide by it where practical. Currently, we do abide by this pattern (after a fashion) with Sample:

>>> client.new_sample(name='...')
Sample(name='...')

If we include any of the read-only traits in the __repr__ output, then we'll be showing something that the user isn't allowed to specify. That is, if the user sees

>>> Sample(name='...', revision=...)

and then, mirroring the pattern above, tries to type:

>>> sample = client.new_sample(name='...', revision=...)

an error will be raised, because the revision of a new Sample is always 0 and is not a parameter.

On the other hand, it's handy to see the revision number in the output. I think we should probably include it and break the pattern. What do you think, @ke-zhang-rd? Any thoughts, @DanOlds, @EliotGann?

@EliotGann
Copy link

Breaking the pattern seems like what I would expect. If you can communicate read-only or system-controlled properties in a different way it would make sense to me. (UID is one that shouldn't be changed, I think)

@ke-zhang-rd
Copy link
Author

I see your point @danielballan. We may need break pattern.
User may expect a connection(mapping) between sample's actual info(IUCr_chemical_formula='', composition='', description='')and revision. I feel printing them together is a intuitive way to show this mapping.

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

No branches or pull requests

3 participants