telegraphic / pygdsm

Python interface to Global Diffuse Sky Models (GDSM) for the radio sky between 10 MHz - 5 THz
MIT License
30 stars 7 forks source link

Allow for an artificial horizon when generating an *Observer sky map. #25

Open David-McKenna opened 7 months ago

David-McKenna commented 7 months ago

This adds a kwarg to BaseObserver.generate() to allow for the horizon to be set at above 0 degrees, allowing for only a fraction of the visible sky to be sampled.

Additionally, I have added tests to ensure that only a fraction of the sky is visible, and that the rad/deg convention used by pyephem for the lon/lat values is also respected for the elevation keyword.

what-the-diff[bot] commented 7 months ago

PR Summary

Overall, this PR provides an important update to sky observation functionalities, allowing for an elevation parameter to be considered, and extends test coverage, making our codebase more robust.

David-McKenna commented 7 months ago

Platform dependent masking fraction? Oh, boy, that's going to be fun to figure out...

telegraphic commented 7 months ago

LGTM! FWIW, I would be happy to merge w/o a perfect test as the masking fraction issue is very strange. I've just updated the codecov CODECOV_TOKEN secret, so if you merge from master it should pass tests. (Also happy to merge w/o that test passing).

David-McKenna commented 7 months ago

I'd like to find some way of making it less flakey -- whether it's by using isclose like you mentioned the other day, or by finding a value that reduces the noise/jitter between platforms (i.e., a high elevation where there is a lower fraction of masked pixels).

I'll hopefully have time to find some kind of resolution for this later this week.

David-McKenna commented 7 months ago

While watching the checks execute I thought I might as well highlight that while you're linting with flake8, the errors are actually being ignored by the workflow, I'll see if I can patch that up in a bit.

Looks like that's intentional, https://github.com/telegraphic/pygdsm/blob/master/.github/workflows/python-app.yml#L37

David-McKenna commented 7 months ago

Looks like CodeCov is out of action again, but the tests were successful this time