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

should RMG's __repr__ methods output default values? #1132

Closed
goldmanm opened this issue Aug 7, 2017 · 3 comments
Closed

should RMG's __repr__ methods output default values? #1132

goldmanm opened this issue Aug 7, 2017 · 3 comments
Labels
abandoned abandoned issue/PR as determined by actions bot stale stale issue/PR as determined by actions bot

Comments

@goldmanm
Copy link
Contributor

goldmanm commented Aug 7, 2017

This is a question moved from #1112 for discussion. Currently RMG's __repr__ methods only output non-default values for class attributes. Should they include all of them?

@mjohnson541
Copy link
Contributor

While, I don't think this is a huge deal. As I said before on #1112, my opinion is that we shouldn't assume that the user printing the object out knows what the defaults are, and for most objects I don't think the extra printed out text ~10% more (roughly I think?) in the repr methods is all that annoying. I think it saves someone a few minutes checking what the default attributes are and that they're only printed if they're not default. I don't mean to argue that it's worth our time to change them, but I don't think it hurts us as much as it helps us to print them out.

@rwest
Copy link
Member

rwest commented Aug 7, 2017

My inclination would have been towards the other approach: have the __repr__ produce the cleanest and briefest possible output that would reproduce the original object when evaluated. Also in a way reminds people that there are defaults.
But I don't feel too strongly and maybe could be convinced - especially if the 10% estimate isn't far off.

How many instances of this are we talking about anyway?

Possibly there's a middle ground - things that get printed rarely, like a reaction system, include them all. Things that get printed a ton (like entries in a library) keep them brief?

(pedantic check: I think @goldmanm means data attributes on instances, not Class attributes? )

@github-actions
Copy link

This issue is being automatically marked as stale because it has not received any interaction in the last 90 days. Please leave a comment if this is still a relevant issue, otherwise it will automatically be closed in 30 days.

@github-actions github-actions bot added the stale stale issue/PR as determined by actions bot label Jun 22, 2023
@github-actions github-actions bot added the abandoned abandoned issue/PR as determined by actions bot label Jul 24, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
abandoned abandoned issue/PR as determined by actions bot stale stale issue/PR as determined by actions bot
Projects
None yet
Development

No branches or pull requests

3 participants