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

Validate channel inputs - [merged] #212

Closed nabobalis closed 10 months ago

nabobalis commented 3 years ago

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

Merges channel-validation -> master

Fixes https://gitlab.com/LMSAL_HUB/aia_hub/aiapy/-/issues/57

I successfully made a decorator! I think this is the right approach. If it is, then I can look at rolling it out to the rest of the codebase.

nabobalis commented 3 years ago

In GitLab by @wtbarnes on Oct 6, 2020, 15:01

This is great! I think this could be applied a few other places other than channel as well. Just one question...where is the decorator? :sweat_smile:

nabobalis commented 3 years ago

In GitLab by @dstansby1 on Oct 7, 2020, 02:03

added 1 commit

Compare with previous version

nabobalis commented 3 years ago

In GitLab by @dstansby1 on Oct 7, 2020, 02:03

:rolling_on_the_floor_laughing: should be there now

nabobalis commented 3 years ago

In GitLab by @codecov on Oct 7, 2020, 02:10

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@7602268). Click here to learn what that means. The diff coverage is 95.23%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master      #88   +/-   ##
=========================================
  Coverage          ?   97.11%           
=========================================
  Files             ?       15           
  Lines             ?      451           
  Branches          ?        0           
=========================================
  Hits              ?      438           
  Misses            ?       13           
  Partials          ?        0           
Impacted Files Coverage Δ
aiapy/util/decorators.py 94.73% <94.73%> (ø)
aiapy/response/channel.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 7602268...96ec257. Read the comment docs.

nabobalis commented 3 years ago

In GitLab by @dstansby1 on Oct 7, 2020, 05:43

added 1 commit

Compare with previous version

nabobalis commented 3 years ago

In GitLab by @dstansby1 on Oct 7, 2020, 05:44

I think I found everything that takes a channel as input. I originally wrote the decorator to allow an arbitrary argument name, and specification of a custom channel list, but I think everywhere uses 'channel' as an argument name and allows any of the valid channels, so I could get rid of these generalisations?

nabobalis commented 3 years ago

In GitLab by @wtbarnes on Oct 7, 2020, 07:17

Commented on aiapy/psf/psf.py line 156

@validate_channel('channel', valid_channels=[94, 131, 171, 193, 211, 304, 335]*u.angstrom)

I think this is the only case where not all channels are valid.

nabobalis commented 3 years ago

In GitLab by @wtbarnes on Oct 7, 2020, 07:17

Commented on aiapy/calibrate/util.py line 107

This check is now redundant and can be removed. The associate test for this is also what is causing the CI to fail. It can be removed as well.

nabobalis commented 3 years ago

In GitLab by @wtbarnes on Oct 7, 2020, 07:21

I think keeping it general is good. The custom channel list is actually applicable in the aiapy.psf.psf function since there is no PSF for the UV and visible channels. I also prefer having to explicitly state what argument is being validated.

nabobalis commented 3 years ago

In GitLab by @dstansby1 on Oct 7, 2020, 08:48

Commented on aiapy/psf/psf.py line 156

changed this line in version 4 of the diff

nabobalis commented 3 years ago

In GitLab by @dstansby1 on Oct 7, 2020, 08:48

added 1 commit

Compare with previous version

nabobalis commented 3 years ago

In GitLab by @dstansby1 on Oct 7, 2020, 08:52

Commented on aiapy/calibrate/util.py line 107

changed this line in version 5 of the diff

nabobalis commented 3 years ago

In GitLab by @dstansby1 on Oct 7, 2020, 08:52

added 1 commit

Compare with previous version

nabobalis commented 3 years ago

In GitLab by @dstansby1 on Oct 7, 2020, 08:53

resolved all threads

nabobalis commented 3 years ago

In GitLab by @wtbarnes on Oct 7, 2020, 10:25

approved this merge request

nabobalis commented 3 years ago

In GitLab by @wtbarnes on Oct 7, 2020, 10:25

unapproved this merge request

nabobalis commented 3 years ago

In GitLab by @wtbarnes on Oct 7, 2020, 10:26

approved this merge request

nabobalis commented 3 years ago

In GitLab by @wtbarnes on Oct 7, 2020, 10:26

mentioned in commit a0ae95d8ca17a3ae5c893b9f6366e5746664ac6e