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

Centralize __repr__ and __str__ in base Region class #101

Merged
merged 5 commits into from
Jan 25, 2017

Conversation

larrybradley
Copy link
Member

The PR also defines separate __repr__ and __str__.

Example:

>>> from regions import PixCoord, EllipsePixelRegion
>>> cen  = PixCoord(10, 20)
>>> reg = EllipsePixelRegion(cen, major=5., minor=3., angle=45.)
>>> reg
<EllipsePixelRegion(PixCoord(x=10, y=20), major=5.0, minor=3.0, angle=45.0)>

>>> print(reg)
Region: EllipsePixelRegion
center: PixCoord(x=10, y=20)
major: 5.0
minor: 3.0
angle: 45.0

@larrybradley larrybradley added this to the 0.2 milestone Jan 20, 2017
Copy link
Member

@cdeil cdeil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great!
+1 to merge if tests pass.

The multi-line strings in the tests with the arbitrary wrapping aren't pleasant on the eyes. But I don't know a better way to do it, so unless someone else has an alternative, possibly nicer suggestion, no need to discuss that point.

@larrybradley
Copy link
Member Author

@cdeil I think the only alternative is to leave them as single long (wrapped) lines.

@cdeil
Copy link
Member

cdeil commented Jan 20, 2017

I think the only alternative is to leave them as single long (wrapped) lines.

Yes, that's what I've used before mostly. But now that I see how you've done it here, I think I prefer your solution. (no vertical scrolling while reading it)

@bsipocz
Copy link
Member

bsipocz commented Jan 25, 2017

Tests are passing, and as @cdeil has already approved above I'm merging.

@bsipocz bsipocz merged commit ffba0b9 into astropy:master Jan 25, 2017
@larrybradley larrybradley deleted the repr branch January 25, 2017 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants