From f62e8bfad30060493c220ae04292e747533d4bd7 Mon Sep 17 00:00:00 2001 From: Simon Walker Date: Tue, 5 Jul 2022 22:42:21 +0100 Subject: [PATCH 1/5] feat: add (ignored) failing test --- fitsio/src/headers.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/fitsio/src/headers.rs b/fitsio/src/headers.rs index c6ccbd6a..cecf185d 100644 --- a/fitsio/src/headers.rs +++ b/fitsio/src/headers.rs @@ -263,4 +263,23 @@ mod tests { assert!(res); } + + #[test] + #[ignore] + fn different_types_have_same_value() { + // https://github.com/mindriot101/rust-fitsio/issues/167 + + let mut f = FitsFile::open("../testdata/full_example.fits").unwrap(); + let hdu = f.primary_hdu().unwrap(); + + let float_val = dbg!(hdu.read_key::(&mut f, "INTTEST").unwrap()); + let double_val = dbg!(hdu.read_key::(&mut f, "INTTEST").unwrap()); + let int_val = dbg!(hdu.read_key::(&mut f, "INTTEST").unwrap()); + let long_val = dbg!(hdu.read_key::(&mut f, "INTTEST").unwrap()); + + assert_eq!(float_val, 42.0_f32); + assert_eq!(double_val, 42.0_f64); + assert_eq!(int_val, 42i32); + assert_eq!(long_val, 42i64); + } } From 0159c3e4335f4266a81f55a1737ab731a660ea9f Mon Sep 17 00:00:00 2001 From: Simon Walker Date: Sun, 10 Jul 2022 12:35:01 +0100 Subject: [PATCH 2/5] fix: read i32 value as i64 then cast We should not use `fits_read_key_log` since this reads a "logical" i.e. boolean. --- fitsio/src/headers.rs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/fitsio/src/headers.rs b/fitsio/src/headers.rs index cecf185d..50b72d0d 100644 --- a/fitsio/src/headers.rs +++ b/fitsio/src/headers.rs @@ -50,7 +50,6 @@ macro_rules! reads_key_impl { }; } -reads_key_impl!(i32, fits_read_key_log); #[cfg(all(target_pointer_width = "64", not(target_os = "windows")))] reads_key_impl!(i64, fits_read_key_lng); #[cfg(any(target_pointer_width = "32", target_os = "windows"))] @@ -58,6 +57,19 @@ reads_key_impl!(i64, fits_read_key_lnglng); reads_key_impl!(f32, fits_read_key_flt); reads_key_impl!(f64, fits_read_key_dbl); +// Special case reading i32 values, because cfitsio does not have a function for reading "short int" +// keys, only "int" keys i.e. there is no cfitsio function that reads a 32 bit integer key from a +// header. +impl ReadsKey for i32 { + fn read_key(f: &mut FitsFile, name: &str) -> Result + where + Self: Sized, + { + let i64_value = i64::read_key(f, name)?; + Ok(i64_value as _) + } +} + impl ReadsKey for bool { fn read_key(f: &mut FitsFile, name: &str) -> Result where @@ -265,7 +277,6 @@ mod tests { } #[test] - #[ignore] fn different_types_have_same_value() { // https://github.com/mindriot101/rust-fitsio/issues/167 From 2e7e85c273bd80f4615421af5423c55c3bee449d Mon Sep 17 00:00:00 2001 From: Simon Walker Date: Sun, 10 Jul 2022 12:36:13 +0100 Subject: [PATCH 3/5] fix: reading boolean reads i64 So we don't have to perform a cast as well as the comparison. --- fitsio/src/headers.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fitsio/src/headers.rs b/fitsio/src/headers.rs index 50b72d0d..60fd480b 100644 --- a/fitsio/src/headers.rs +++ b/fitsio/src/headers.rs @@ -75,7 +75,7 @@ impl ReadsKey for bool { where Self: Sized, { - let int_value = i32::read_key(f, name)?; + let int_value = i64::read_key(f, name)?; Ok(int_value > 0) } } From 30a0d680a75497a77e258cb3d6b6933c68da4449 Mon Sep 17 00:00:00 2001 From: Simon Walker Date: Sun, 10 Jul 2022 18:56:27 +0100 Subject: [PATCH 4/5] feat: support writing boolean header values This is needed to test _reading_ boolean header values of both true and false. --- fitsio/src/headers.rs | 44 +++++++++++++++++++++++++++++++++++++++---- fitsio/src/longnam.rs | 12 +++++++++++- 2 files changed, 51 insertions(+), 5 deletions(-) diff --git a/fitsio/src/headers.rs b/fitsio/src/headers.rs index 60fd480b..efc38af0 100644 --- a/fitsio/src/headers.rs +++ b/fitsio/src/headers.rs @@ -104,6 +104,15 @@ impl ReadsKey for String { } /// Writing a fits keyword +/// This is currently limited to types: +/// +/// * bool +/// * i32 +/// * i64 +/// * f32 +/// * f64 +/// * String +/// * &'_ str pub trait WritesKey { #[doc(hidden)] fn write_key(f: &mut FitsFile, name: &str, value: Self) -> Result<()>; @@ -169,6 +178,23 @@ macro_rules! writes_key_impl_flt { writes_key_impl_flt!(f32, fits_write_key_flt); writes_key_impl_flt!(f64, fits_write_key_dbl); +impl WritesKey for bool { + fn write_key(f: &mut FitsFile, name: &str, value: Self) -> Result<()> { + let c_name = ffi::CString::new(name)?; + let mut status = 0; + unsafe { + fits_write_key_log( + f.fptr.as_mut() as *mut _, + c_name.as_ptr(), + value as i32, + ptr::null_mut(), + &mut status, + ); + } + check_status(status) + } +} + impl WritesKey for String { fn write_key(f: &mut FitsFile, name: &str, value: Self) -> Result<()> { WritesKey::write_key(f, name, value.as_str()) @@ -268,12 +294,22 @@ mod tests { #[test] fn boolean_header_values() { - let mut f = FitsFile::open("../testdata/full_example.fits").unwrap(); - let hdu = f.primary_hdu().unwrap(); + duplicate_test_file(|filename| { + // add some boolean headers + { + let mut f = FitsFile::edit(filename).unwrap(); + let hdu = f.primary_hdu().unwrap(); + hdu.write_key(&mut f, "TVAL", true).unwrap(); + hdu.write_key(&mut f, "FVAL", false).unwrap(); + } - let res = dbg!(hdu.read_key::(&mut f, "SIMPLE").unwrap()); + // now assert the values read back the same + let mut f = FitsFile::open(filename).unwrap(); + let hdu = f.primary_hdu().unwrap(); - assert!(res); + assert_eq!(hdu.read_key::(&mut f, "TVAL").unwrap(), true); + assert_eq!(hdu.read_key::(&mut f, "FVAL").unwrap(), false); + }); } #[test] diff --git a/fitsio/src/longnam.rs b/fitsio/src/longnam.rs index 9e9ba871..a39a58ea 100644 --- a/fitsio/src/longnam.rs +++ b/fitsio/src/longnam.rs @@ -9,7 +9,7 @@ pub(crate) use crate::sys::{ ffgcvi, ffgcvj, ffgcvjj, ffgcvk, ffgcvs, ffgcvui, ffgcvuj, ffgcvujj, ffgcvuk, ffghdn, ffghdt, ffgidm, ffgiet, ffgisz, ffgkyd, ffgkye, ffgkyj, ffgkyjj, ffgkyl, ffgkys, ffgncl, ffgnrw, ffgpv, ffgsv, fficol, ffinit, ffmahd, ffmnhd, ffopen, ffpcl, ffpcls, ffphps, ffpky, ffpkyd, ffpkye, - ffpkys, ffppr, ffpss, ffrsim, ffthdu, fitsfile, LONGLONG, + ffpkyl, ffpkys, ffppr, ffpss, ffrsim, ffthdu, fitsfile, LONGLONG, }; #[cfg(feature = "default")] pub use libc::{ @@ -548,6 +548,16 @@ pub(crate) unsafe fn fits_write_key_str( ffpkys(fptr, keyname, value, comm, status) } +pub(crate) unsafe fn fits_write_key_log( + fptr: *mut fitsfile, + keyname: *const c_char, + value: c_int, + comm: *const c_char, + status: *mut c_int, +) -> c_int { + ffpkyl(fptr, keyname, value, comm, status) +} + pub(crate) unsafe fn fits_write_img( fptr: *mut fitsfile, datatype: c_int, From 584c18aef89af86338b487c21b0393a082b8c0af Mon Sep 17 00:00:00 2001 From: Simon Walker Date: Sun, 10 Jul 2022 18:57:01 +0100 Subject: [PATCH 5/5] doc: update docstring for ReadsKey --- fitsio/src/headers.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/fitsio/src/headers.rs b/fitsio/src/headers.rs index efc38af0..8ee121f0 100644 --- a/fitsio/src/headers.rs +++ b/fitsio/src/headers.rs @@ -13,6 +13,7 @@ Trait applied to types which can be read from a FITS header This is currently: +* bool * i32 * i64 * f32