Functionality of new geodetic_skymap

[CassandraAuri]

Hey Mike,

I am just at the point of my research where I need to consider emission heights. Is the geodetic_skymap stable and functional or should I wait until more testing is done.

Clear Skies and Happy Trails,
Cassandra

[mshumko]

Hi Cassandra,

There may be future modifications, but for now it is stable and I wrote a test for it. It currently maps onto a spherical Earth. In the future I'd like to figure out how to map it to the oblate spheroid. Thus, there is a small discrepancy between the official skymaps produced by U Calgary and geodetic_skymap().

[mshumko]

That is a good idea! I initially only envisioned using geodetic_skymap() for ASIs that do not provide them, but I can see this utility in studies like yours.

My only hesitation is implicitly overriding the official skymaps and then finding out later that the skymap discrepancies are larger than we're comfortable with. One solution is to make toggling between the skymap methods explicit: add a new kwarg, custom_alt=False to the loaders (asilib.asi.themis, asilib.asi.rego, etc.).

Then the three conditional branches are:

1. if custom_alt=False and the user chooses an official altitude, then the Calgary skymaps are used;
2. if custom_alt=False and the user chooses a custom altitude, then raise an error as is currently done, but add a hint that the custom_alt kawrg exists;
3. if custom_alt=True calculate the custom skymap via geodetic_skymap() regardless of the altitude chosen.

I can add this functionality, but as you already eluded, it may take longer than you'd like. You're welcome to implement it, add the kwarg description to the docstrings, and submit a PR request!

Here is a code snippet on how I do it for an imager that is not officially part of asilib:

skymap['lat'], skymap['lon'] = asilib.skymap.geodetic_skymap(
            (meta['lat'], meta['lon'], meta['alt']),
            skymap['az'],
            skymap['el'],
            alt
            )

Also, please add a few tests that ensures that future changes don't break yours. Please let me know if you need ideas or guidance on how to write them (e.g., should you compare skymap arrays directly vs comparing plot outputs as many of my tests do already)

[CassandraAuri]

Sounds good to me. I will implement the keyword. Additionally, it will be very helpful to my python experience to learn more about testing as I have just been using code snippets from my GUI to test everything I've implemented which is not sustainable for troubleshooting. I will see if I can't get this done before school starts on the 5th, I will update this forum if I can't complete it by then.

[mshumko]

Thanks! Perhaps example tests are the easiest way to learn...

1. Here is a test that compares the output plot to a reference plot in the `tests/baseline_images directory'. See the matplotlib image comparison docs for more info.
2. And here is a test that does array comparison.

While I do not do this often myself, I prefer to have more option 2 tests (unless I am testing an example plot).

P.S. Since you are just getting into this, I will be thankful if you help edit the Contribute section of the documentation. The current docs are not thorough enough and maybe confusing, so I think that you can do a much better job since you have a learner's perspective.

[CassandraAuri]

Documenting: I have added support but still need to do testing. It seems to work with my data set but I will be creating an integrated test hopefully this weekend. I will also try to work on the contribute section, maybe through dictation first.

[mshumko]

Hi @CassandraAuri,

Your plan sounds good. No worries on updating everything at once. The two important things are: the docstrings accurately describe the new kwargs and behavior, and the new code is executed by at least one test (and all other tests pass). asilib uses pytest which makes writing and running tests very simple.