-
Notifications
You must be signed in to change notification settings - Fork 5
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
Add custom healpix plotting method #374
base: main
Are you sure you want to change the base?
Conversation
Bumps [pre-commit-ci/lite-action](https://github.com/pre-commit-ci/lite-action) from 1.0.2 to 1.0.3. - [Release notes](https://github.com/pre-commit-ci/lite-action/releases) - [Commits](pre-commit-ci/lite-action@v1.0.2...v1.0.3) --- updated-dependencies: - dependency-name: pre-commit-ci/lite-action dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]>
…ons/pre-commit-ci/lite-action-1.0.3 Bump pre-commit-ci/lite-action from 1.0.2 to 1.0.3
…o-type update healpixdataset catalog info type
Click here to view all benchmarks. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## development #374 +/- ##
===============================================
- Coverage 93.99% 93.75% -0.24%
===============================================
Files 58 59 +1
Lines 2130 2241 +111
===============================================
+ Hits 2002 2101 +99
- Misses 128 140 +12 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work, Sean!
# Set projection | ||
_set_wcs(ax, wcs) | ||
|
||
return ax |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need it to return anything? Maybe it is better to be col
? (should plot_healpix_map
also return col
?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like plot_healpix_map
returning ax, it makes it easy to overplot things, and you can get the collection from it anyway. You're right though that the inner function doesn't really need to return anything, I'll update that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also like that it matches plt.subplots fig, ax = plt.subplots()
return ipix_d | ||
|
||
|
||
def plot_healpix_map( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, we should cull the pixels outside of the fov, but I was lazy and I didn't think about this use case so I didn't think it would impact anything other than performance. But this is a good idea to add.
@smcguire-cmu I played more! It gave me empty plot when I tried to do it for ALLWISE: import hipscat
catalog = hipscat.loaders.read_from_hipscat('https://data.lsdb.io/unstable/wise/allwise')
hipscat.inspection.plot_points(catalog) It also uses a lot of RAM, but it is probably expected? Zoomed version works fine import hipscat
from astropy.coordinates import Angle, SkyCoord
catalog = hipscat.loaders.read_from_hipscat('https://data.lsdb.io/unstable/wise/allwise')
hipscat.inspection.plot_points(catalog, center=SkyCoord(0, 0, unit='deg'), fov=(Angle(5, 'deg'), Angle(10, 'deg'))) |
@smcguire-cmu One more thing. It would be nice to make a "view window" to be a rectangle when |
Co-authored-by: Konstantin Malanchev <[email protected]>
Replaces healpy's plotting methods with a new healpix map plotting method based on mocpy's plotting functions.
Fixes #242