search

LM-SAL / aiapy

Python library for AIA data analysis
https://aiapy.readthedocs.io/en/stable/
BSD 3-Clause "New" or "Revised" License
6 stars 3 forks source link

Inconsistent validation of AIAMap #68

Open nabobalis opened 3 years ago

nabobalis commented 3 years ago

In GitLab by @wtbarnes on Oct 2, 2020, 12:15

In most functions that take in a map, we validate whether it is an AIAMap and throw an error if it is not. This has ressulted in a lot of repeated code and inconsistency. We should have a decorator that handles this for us.

nabobalis commented 3 years ago

In GitLab by @PaulJWright on Oct 2, 2020, 13:18

Would be happy to give this a shot (depending on how urgent this is), or at least be interested in going through the process with whoever decides to do it!

nabobalis commented 3 years ago

In GitLab by @wtbarnes on Oct 2, 2020, 14:38

Have at it! The decorator approach was just my first thought. The easy thing would be just going around and making sure we are always checking whether it is an AIAMap. But having all of that if not isinstance then throw exception code all over the place is just kind of messy.

nabobalis commented 3 years ago

In GitLab by @wtbarnes on Oct 2, 2020, 14:53

Also should make sure to update docstrings. Right now we have a mix of sunpy.map.Map and sunpy.map.sources.sdo.AIAMap. I don't have a strong opinion about which we use, but we should be consistent.

nabobalis commented 3 years ago

In GitLab by @dstansby1 on Oct 6, 2020, 13:04

@PaulJWright if you're looking for some decorator pointers, I think my merge request in https://gitlab.com/LMSAL_HUB/aia_hub/aiapy/-/merge_requests/88 contains a very similar structure to a decorator that validates input maps.

nabobalis commented 3 years ago

In GitLab by @PaulJWright on Oct 6, 2020, 13:11

Sounds good. I think your PR is missing aiapy.util.decorators, or at least the validate channel function. I'll give it a shot once the PR is updated. What would be the best way? Wait until the PR is merged before trying?

nabobalis commented 3 years ago

In GitLab by @PaulJWright on Oct 6, 2020, 19:00

Yeah it seems the PR is missing the decorator. Once it's added, I'll give it a shot at making a decorator for this. I'll be sure to ping you both if I can't figure it out

nabobalis commented 3 years ago

In GitLab by @PaulJWright on Nov 13, 2020, 16:13

I think @karthik_v wants to contribute here, no pressure Karthik, but FYI to everyone above!

nabobalis commented 3 years ago

In GitLab by @wtbarnes on Apr 30, 2021, 05:41

changed health status to needs attention