Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENH: Add conditional to adapt behaviour depending on astropy version and warn user #73

Merged
merged 19 commits into from
Nov 3, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 29 additions & 11 deletions amical/mf_pipeline/bispect.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,15 @@
"""
import sys
import time
import warnings

import astropy
import numpy as np
from astropy.io import fits
from matplotlib import pyplot as plt
from mpl_toolkits.axes_grid1 import make_axes_locatable
from munch import munchify as dict2class
from packaging.version import Version
from scipy.optimize import minimize
from termcolor import cprint
from tqdm import tqdm
Expand All @@ -38,6 +41,9 @@
from amical.tools import cov2cor


ASTROPY_VERSION = Version(astropy.__version__)


def _compute_complex_bs(
ft_arr,
index_mask,
Expand Down Expand Up @@ -985,19 +991,31 @@ def _add_infos_header(infos, hdr, mf, pa, filename, maskname, npix):
infos["maskname"] = maskname
infos["isz"] = npix

# HACK: astropy _HeaderCommentaryCards are registered as mappings,
# so munch tries to access their keys, leading to attribute error
# to prevent this, we remove commentary cards as a temporary fix.
# (As of June 23 2021, with astropy version 4.2.1)
# See:
# https://github.com/SydneyAstrophotonicInstrumentationLab/AMICAL/issues/31
# https://github.com/astropy/astropy/issues/11866
# Raise a warning that old astropy version drop commentary cards, but only if
# there are any in the original header
hdr_commentary_keys = fits.Card._commentary_keywords
hdr = hdr.copy()
for key in hdr_commentary_keys:
hdr.remove(key, ignore_missing=True, remove_all=True)
if any(hck in hdr for hck in hdr_commentary_keys) and (
ASTROPY_VERSION < Version("5.0rc")
):
warnings.warn(
"Commentary cards are removed from the header with astropy"
f" version < 5.0. Your astropy version is"
f" {ASTROPY_VERSION}",
RuntimeWarning,
)
# HACK: astropy _HeaderCommentaryCards are registered as mappings,
# so munch tries to access their keys, leading to attribute error
# to prevent this, we remove commentary cards as a temporary fix.
# (As of September 2 2021, with astropy version 4.3.1)
# See:
# https://github.com/SydneyAstrophotonicInstrumentationLab/AMICAL/issues/31
# https://github.com/astropy/astropy/issues/11866
# Resolved upstream with
# https://github.com/astropy/astropy/pull/11923
hdr = hdr.copy()
for key in hdr_commentary_keys:
hdr.remove(key, ignore_missing=True, remove_all=True)

# Now that header is compatible with munch, we add it to infos
infos["hdr"] = hdr

# Save keys of the original header (as needed):
Expand Down
70 changes: 59 additions & 11 deletions amical/tests/test_extraction.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,32 @@
import astropy
import munch
import pytest
from astropy.io import fits
from packaging.version import Version

from amical.mf_pipeline.bispect import _add_infos_header


# Astropy versions for
ASTROPY_VERSION = Version(astropy.__version__)


@pytest.fixture()
def commentary_infos():

# Add hdr to infos placeholders for everything but hdr
mf = munch.Munch(pixelSize=1.0)

# SimulatedData avoids requiring extra keys in infos
infos = munch.Munch(orig="SimulatedData", instrument="unknown")

# Create a fits header with commentary card
hdr = fits.Header()
hdr["HISTORY"] = "History is a commentary card"

return _add_infos_header(infos, hdr, mf, 1.0, "afilename", "amaskname", 1)


def test_add_infos_simulated():
# Ensure that keys are passed to infos for simulated data, but only when available

Expand All @@ -12,31 +35,56 @@ def test_add_infos_simulated():
hdr["DATE-OBS"] = "2021-06-23"
hdr["TELESCOP"] = "FAKE-TEL"

# This test is for simulated data only
# SimulatedData avoids requiring extra keys in infos
infos = munch.Munch(orig="SimulatedData", instrument="unknown")

# Add hdr to infos placeholders for everything but hdr
mf = munch.Munch(pixelSize=1.0)
infos = _add_infos_header(infos, hdr, mf, 1.0, "afilename", "amaskname", 1)

# Check that we kept required keys
assert infos["date-obs"] == hdr["DATE-OBS"]
assert infos["telescop"] == hdr["TELESCOP"]
assert "observer" not in infos # Keys that are not in hdr should not be in infos

# Keys that are not in hdr should not be in infos or hdr
assert "observer" not in infos
assert "observer" not in infos.hdr

def test_add_infos_header_commentary():

@pytest.mark.filterwarnings("ignore: Commentary cards")
vandalt marked this conversation as resolved.
Show resolved Hide resolved
def test_add_infos_header_commentary(commentary_infos):
# Make sure that _add_infos_header handles _HeaderCommentaryCards from astropy

# Create a fits header with commentary card
hdr = fits.Header()
hdr["HISTORY"] = "History is a commentary card"
# Convert everything to munch object
munch.munchify(commentary_infos)

# SimulatedData avoids requiring extra keys in infos
infos = munch.Munch(orig="SimulatedData", instrument="unknown")

@pytest.mark.xfail(
ASTROPY_VERSION < Version("5.0rc"),
reason="Munch cannot handle commentary cards for Astropy < 5.0",
)
def test_commentary_infos_keep(commentary_infos):
assert "HISTORY" in commentary_infos.hdr
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not understand why Codecov reports this as not hit. It runs on my machine whatever version of astropy I use. Any clue ? (not needed to get this merged)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No sorry... Works on my machine as well, and I don't know much about Codecov



@pytest.mark.xfail(
ASTROPY_VERSION > Version("5.0rc"),
reason="Astropy > 5.0 should not raise a warning for commentary cards",
)
def test_no_commentary_warning_astropy_version():

# Add hdr to infos placeholders for everything but hdr
mf = munch.Munch(pixelSize=1.0)
infos = _add_infos_header(infos, hdr, mf, 1.0, "afilename", "amaskname", 1)

# Convert everything to munch object
munch.munchify(infos)
# SimulatedData avoids requiring extra keys in infos
infos = munch.Munch(orig="SimulatedData", instrument="unknown")

# Create a fits header with commentary card
hdr = fits.Header()
hdr["HISTORY"] = "History is a commentary card"

with pytest.warns(
RuntimeWarning,
match="Commentary cards are removed from the header with astropy",
):
infos = _add_infos_header(infos, hdr, mf, 1.0, "afilename", "amaskname", 1)
1 change: 1 addition & 0 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ install_requires =
matplotlib
munch
numpy
packaging>=21.0
scipy
termcolor
tqdm
Expand Down