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

__repr__ for vso.attrs.Wave #786

Merged
merged 14 commits into from
Feb 3, 2014
Merged

__repr__ for vso.attrs.Wave #786

merged 14 commits into from
Feb 3, 2014

Conversation

PritishC
Copy link
Member

@PritishC PritishC commented Feb 1, 2014

I've added a repr method similar to the one the Time class has, in the same package. This should resolve issue #532 that derdon had opened.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 8cc991a on VaticanCameos:master into 6207c54 on sunpy:master.

@derdon
Copy link
Member

derdon commented Feb 1, 2014

Two tests are commented out and I proposed on IRC to use {2!r} instead of {2} to keep it consistent with Time.__repr__ and the classes which inherit from _VSOSimpleAttr.__repr__. Maybe even use {0:d} and {1:d} to make sure that min and max are integers.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) when pulling 03a2fb4 on VaticanCameos:master into 6207c54 on sunpy:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) when pulling 2247718 on VaticanCameos:master into 6207c54 on sunpy:master.

@Cadair
Copy link
Member

Cadair commented Feb 2, 2014

Looks good to me. :)

@derdon
Copy link
Member

derdon commented Feb 2, 2014

The @pytest.fixture decorator does not make any sense here.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 320f1ef on VaticanCameos:master into 6207c54 on sunpy:master.

ehsteve added a commit that referenced this pull request Feb 3, 2014
@ehsteve ehsteve merged commit 0358fc7 into sunpy:master Feb 3, 2014
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 this pull request may close these issues.

None yet

5 participants