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

Improvement in Plotting methods. #194

Merged
merged 20 commits into from Jul 20, 2018

Conversation

Projects
None yet
2 participants
@sushobhana
Member

sushobhana commented Jun 29, 2018

added as_patch for annulus.

In [15]: reg3 = CircleAnnulusPixelRegion(PixCoord(.4,.5), .2 , .3)

In [16]: reg3.plot()
Out[16]: <matplotlib.axes._subplots.AxesSubplot at 0x7efd8f016390>

In [17]: plt.show()

new_annulus

Now, also uses visual meta data for plotting:

In [3]: st = 'image\npoint(1.3,1.5) # point=arrow 14 color=magenta'

In [4]: reg = DS9Parser(st)

In [5]: reg = reg.shapes.to_regions()[0]

In [6]: reg.plot()
Out[6]: <matplotlib.axes._subplots.AxesSubplot at 0x7fdd8e37a2e8>

In [7]: plt.show()

symbol

In [3]: st = 'image\ncircle(1.4,1.5,.3) # color=pink width=3 font="times 10 normal roman" text={Circle} tag={foo} tag={foo bar} This is a Comment'

In [4]: reg = DS9Parser(st)

In [5]: reg = reg.shapes.to_regions()[0]

In [6]: ax = reg.plot()

In [7]: ax.legend()
Out[7]: <matplotlib.legend.Legend at 0x7f7e4c1aa358>

In [8]: plt.show()

n1

In [3]: a = 'image\ntext(1315,1041) # color=magenta font="helvetica 14 bold roman" text={Region}'

In [4]: reg = DS9Parser(a).shapes.to_regions()[0]

In [5]: ax = reg.plot()

In [6]: ax.set_xlim(0,2000)
Out[6]: (0, 2000)

In [7]: ax.set_ylim(0,2000)
Out[7]: (0, 2000)

In [8]: plt.show()
/home/sushobhana/anaconda3/lib/python3.6/site-packages/matplotlib/font_manager.py:1320: UserWarning: findfont: Font family ['helvetica'] not found. Falling back to DejaVu Sans
  (prop.get_family(), self.defaultFamily[fontext]))

text1

In [7]: from regions import *

In [8]: from matplotlib import pyplot as plt

In [9]: reg3 = RectangleAnnulusPixelRegion(PixCoord(.4,.5), .2 , .3, .4, .5)

In [10]: ax =  reg3.plot()

In [11]: plt.show()

rect_new

Also, implemented the __eq__ for PixCoord.

@sushobhana sushobhana force-pushed the sushobhana:plotting branch 3 times, most recently from 632588c to d2c49c6 Jul 1, 2018

@sushobhana sushobhana referenced this pull request Jul 3, 2018

Merged

Added Text regions #177

@sushobhana sushobhana force-pushed the sushobhana:plotting branch 3 times, most recently from d796309 to a5758e4 Jul 10, 2018

@keflavich

This is looking really good. I'm not sure I caught everything in this review; there's a lot changed here.

Could you add an example to the docs with these improved plots? Something like the images on the frontpage of the pyregion docs. That might be best as the topic of another PR, but we should show off these changes somewhere!

patch = mpatches.PathPatch(path, **kwargs)
return patch
else:
raise NotImplementedError

This comment has been minimized.

@keflavich

keflavich Jul 11, 2018

Collaborator

Is this supposed to be reachable? If not, see if you can add a message that might help the user figure out what went wrong. I don't believe non-concentric annuli should be possible to declare.

This comment has been minimized.

@sushobhana

sushobhana Jul 12, 2018

Member

yes, it's reachable. It is the CompoundPixelRegion class and not CircleAnnulusPixelRegion

else:
self._raise_error("Not in proper format: '{0}' should be a symbol".format(y))
elif self.region_type == 'text':
self.meta['text'] = y

This comment has been minimized.

@keflavich

keflavich Jul 11, 2018

Collaborator

can we maybe use more descriptive variable names than x and y here? That might be the subject of a different PR, I just realize now looking at this code that it's hard to understand where those come from with these names.

from matplotlib.patches import Arrow
x = self.start.x
y = self.start.y
dx = self.end.x - self.start.x
dy = self.end.y - self.start.y
if not 'width' in kwargs:
kwargs['width'] = .1 # Let the default width be .1 instead of 1.

This comment has been minimized.

@keflavich

keflavich Jul 11, 2018

Collaborator

Why? I don't mind, just wondering if there's a reason. Does that make it look more like ds9?

This comment has been minimized.

@sushobhana

sushobhana Jul 12, 2018

Member

It does not look like a line with width one. This Arrow patch seems ugly.Also, there is a line meta in DS9 decides which end of the line would be arrow like. I will work on that.

return _to_io_meta(region_meta, region_visual, valid_keys, key_mappings)
if 'font' in meta:
meta['font'] += " {0} {1} {2}".format(shape_meta.get('fontsize', 12),
shape_meta.get('fontstyle', 'normal'),

This comment has been minimized.

@keflavich

keflavich Jul 11, 2018

Collaborator

whitespace (indentation isn't consistent here)

@sushobhana sushobhana force-pushed the sushobhana:plotting branch 2 times, most recently from a5e4460 to 6c512ae Jul 19, 2018

@sushobhana sushobhana force-pushed the sushobhana:plotting branch from 6c512ae to 694f698 Jul 19, 2018

@sushobhana

This comment has been minimized.

Member

sushobhana commented Jul 19, 2018

There are a lot of big and small changes made here, and I don't want to push things further in the PR here.
There is still more to be done to make this more DS9 though.
Up for a final review!
@keflavich

@keflavich

This comment has been minimized.

Collaborator

keflavich commented Jul 20, 2018

I'm happy with this, so I'm merging it. Could you link to the Issues with the next steps? For one, we want a documentation page analogous to pyregion's frontpage showing all the graphical tools in action; it can pretty much be the RST version of the original issue posted here.

You also left a to-do item of having some of the internals of the plot method put into an as_patch method.

@keflavich keflavich merged commit 7dd0951 into astropy:master Jul 20, 2018

3 of 4 checks passed

coverage/coveralls Coverage decreased (-2.04%) to 87.574%
Details
ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment