Closed nabobalis closed 1 year ago
In GitLab by @wtbarnes on Aug 24, 2020, 15:15
Commented on aiapy/calibrate/prep.py line 204
Is 'pixlunit'
a standard FITS keyword? I think the keyword to set here is 'bunit'
. I'd also suggest using 'ct'
rather than 'DN'
as this is what astropy units understands (though the unit of the map is not used anywhere in the map infrastructure so it doesn't matter too much.
In GitLab by @wtbarnes on Aug 24, 2020, 15:15
Commented on aiapy/calibrate/prep.py line 196
I think we can just rely on the derived exposure_time
property here on the map. Map
already does a get
on that FITS keyword so we don't ever have to worry about it being missing. I'd probably just reduce this to checking if the exposure time property is (less than or) equal to 0
In GitLab by @wtbarnes on Aug 24, 2020, 15:16
This looks great! Could you add a test as well? :grin: :grin:
In GitLab by @PaulJWright on Aug 24, 2020, 15:39
Commented on aiapy/calibrate/prep.py line 204
Unclear. bunit
doesn't seem exist in the data when downloaded from JSOC:
res = Fido.search(
attrs.Time('2019-01-01T00:00:10', '2019-01-01T00:00:11'),
attrs.Wavelength(wavemin=335*u.angstrom, wavemax=335*u.angstrom),
attrs.jsoc.Series('aia.lev1_euv_12s'),
attrs.jsoc.Notify('paul@pauljwright.co.uk'))
In GitLab by @wtbarnes on Aug 25, 2020, 11:21
Commented on aiapy/calibrate/prep.py line 204
http://archive.stsci.edu/fits/fits_standard/node40.html#SECTION00942530000000000000
From what I can gather BUNIT
is the standard for array units. If it doesn't already exist, we can always add it in.
In GitLab by @PaulJWright on Aug 25, 2020, 12:13
Commented on aiapy/calibrate/prep.py line 204
BUNIT
doesn't seem to exist (at least in the aia.lev1_euv_12s
series), but will add it in and change pixlunit
too.
In GitLab by @PaulJWright on Aug 25, 2020, 17:02
Commented on aiapy/calibrate/prep.py line 204
changed this line in version 2 of the diff
In GitLab by @PaulJWright on Aug 25, 2020, 17:02
Commented on aiapy/calibrate/prep.py line 196
changed this line in version 2 of the diff
In GitLab by @PaulJWright on Aug 25, 2020, 17:02
added 1 commit
In GitLab by @PaulJWright on Aug 25, 2020, 17:04
Still need to write tests.
In GitLab by @PaulJWright on Aug 25, 2020, 17:04
Commented on aiapy/calibrate/prep.py line 180
is this required?
In GitLab by @codecov on Aug 25, 2020, 17:09
Merging #78 into master will increase coverage by
0.22%
. The diff coverage is100.00%
.
@@ Coverage Diff @@
## master #78 +/- ##
==========================================
+ Coverage 91.02% 91.25% +0.22%
==========================================
Files 14 14
Lines 390 400 +10
==========================================
+ Hits 355 365 +10
Misses 35 35
Impacted Files | Coverage Δ | |
---|---|---|
aiapy/calibrate/prep.py | 97.67% <100.00%> (+0.70%) |
:arrow_up: |
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 75b4aff...3b68b90. Read the comment docs.
In GitLab by @PaulJWright on Aug 25, 2020, 17:18
added 1 commit
In GitLab by @PaulJWright on Aug 25, 2020, 17:58
added 1 commit
In GitLab by @wtbarnes on Aug 28, 2020, 08:59
Commented on aiapy/calibrate/tests/test_prep.py line 135
assert np.all(aia_171_map_norm.data == (aia_171_map.data/aia_171_map.exposure_time.to(u.s).value))
assert aia_171_map_norm.exposure_time == 1.0*u.s
In GitLab by @wtbarnes on Aug 28, 2020, 09:00
Commented on aiapy/calibrate/tests/test_prep.py line 136
Just one last test to always make sure the function is doing what we expect. If this looks good to you, just hit "Apply Suggestion" and it should kick off the pipeline again.
In GitLab by @PaulJWright on Aug 28, 2020, 09:15
added 1 commit
In GitLab by @wtbarnes on Aug 28, 2020, 09:20
Commented on aiapy/calibrate/prep.py line 180
Nope. Only needed when one of the inputs is a astropy.units.Quantity
In GitLab by @wtbarnes on Aug 28, 2020, 09:20
resolved all threads
In GitLab by @wtbarnes on Aug 28, 2020, 09:22
Commented on aiapy/calibrate/tests/test_prep.py line 136
assert aia_171_map_norm.exposure_time == 1.0*u.s
In GitLab by @wtbarnes on Aug 28, 2020, 09:23
Commented on aiapy/calibrate/tests/test_prep.py line 136
Sorry, meant to add this suggestion on the previous one :sweat_smile:
In GitLab by @wtbarnes on Aug 28, 2020, 09:23
resolved all threads
In GitLab by @wtbarnes on Aug 28, 2020, 09:23
Commented on aiapy/calibrate/tests/test_prep.py line 136
changed this line in version 6 of the diff
In GitLab by @wtbarnes on Aug 28, 2020, 09:23
added 1 commit
In GitLab by @wtbarnes on Aug 28, 2020, 09:32
Hmm the test that checks actual normalized data array is now failing...do you know why that might be happening?
In GitLab by @PaulJWright on Aug 28, 2020, 09:59
No, this is bizarre...
_____________________________________________________________________________ test_normexptime ______________________________________________________________________________
aia_171_map = <sunpy.map.sources.sdo.AIAMap object at 0x129cf6400>
SunPy Map
---------
Observatory: SDO
Instrument: AIA 3
Detect...807, 1.26580811],
[ 0.75378418, 0.78479004, 0.8157959 , ..., 1.21514893,
1.25018311, 1.28521729]])
def test_normexptime(aia_171_map):
aia_171_map_norm = normalize_exposure(aia_171_map)
> assert np.all(aia_171_map_norm.data == (aia_171_map.data/aia_171_map.exposure_time.to(u.s).value))
E assert False
E + where False = <function all at 0x10a69d1f0>(array([[-1.67... 0.64254728]]) == array([[-3.35... 1.28521729]])
E + where <function all at 0x10a69d1f0> = np.all
E Use -v to get the full diff)
aiapy/calibrate/tests/test_prep.py:135: AssertionError```
In GitLab by @PaulJWright on Aug 28, 2020, 10:30
added 1 commit
In GitLab by @wtbarnes on Aug 28, 2020, 10:43
approved this merge request
In GitLab by @wtbarnes on Aug 28, 2020, 10:45
mentioned in commit af6a89002bf9588c2ee6ca8c49717a6e1f9cead2
In GitLab by @PaulJWright on Aug 24, 2020, 15:01
_Merges exposurenormalisation -> master
Added
normalize_exposure
function, updated the appropriate example, and included tests. Addresses #27