search

pmelchior / skymapper

Mapping astronomical survey data on the sky, handsomely
MIT License
47 stars 15 forks source link

Fix patches! #8

Closed rainwoodman closed 8 years ago

rainwoodman commented 8 years ago

index

pmelchior commented 8 years ago

That looks a lot better. Do you understand why some patches are lost on one side?

rainwoodman commented 8 years ago

I believe it is related to I didn't duplicate those vertices crossing the boundary. Ideally we can create a path with two disjoint regions, one per side, -- but I am not sure if matplotlib will be able to correctly fill them -- it probably should.

The latest commit adds two convenient functions: histmap and mapshow to the axes object:

a.histmap(ra, dec, weights=FC, nside=64, mean=True, lw=0, edgecolor='none')

rainwoodman commented 8 years ago

or for an existing map:

a.mapshow(m, mask, ....) where mask decides whether to show the pixel or not.

pmelchior commented 8 years ago

OK, great. Two requests:

  1. Move the functionality for histmap and mapshow into a free function, with a thin wrapper for the axes class method.
  2. I'm a bit puzzled about mapshow. See my note there.
rainwoodman commented 8 years ago

I am not so sure about '1'.

These two function are already relatively thin. The real logic is in histogrammap() and _boundary(). Is it the idea we will add the other projection and reuse them? Maybe we want to have them share the same base class instead.

pmelchior commented 8 years ago

I think it's a distinct possibility that we want to another projection. I've done the Lambert Conformal conic already in my old scheme, and the changes are minimal compared to the AEA. So, I think it would be good to avoid duplication of logic that does not depend on specifics of the transformation.

Having a base class for them would be useful. Short of that, you can move the actual logic out into a free method as you have already done with histogrammap() and _boundary().