Skip to content

Commit

Permalink
BinaryTable.addRow(): Fixes to protoype row, and checks for added rows.
Browse files Browse the repository at this point in the history
  • Loading branch information
@attipaci
attipaci committed Dec 7, 2024
1 parent 8198ad2 commit b3de704
Show file tree
Hide file tree
Showing 9 changed files with 158 additions and 125 deletions.
12 changes: 12 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,18 @@ All notable changes to the nom.tam.fits library will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).


## [Unreleased]

Changes coming to the next release, expected around 15 March 2025.

### Fixed

- [#708] `BinaryTable.addRowEntries()` did not work with scalar `Boolean` or `boolean`. (by @attipaci, thanks to @johnmurphyastro)

- [#710] `BinaryTable.addRow()` has some oddities and inconsistencies that resulted in unexpected behavior. (by @attipaci)



## [1.20.2] - 2024-12-01

Minor bug-fix release.
Expand Down
132 changes: 48 additions & 84 deletions src/main/java/nom/tam/fits/BinaryTable.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1826,31 +1826,6 @@ public int addColumn(ColumnDesc descriptor) throws IllegalStateException {
return columns.size();
}

/**
* Converts a boxed table entry to an array.
*
* @param o a boxed table entry or array of some kind
*
* @return an array object that wrap non-array arguments
*
* @throws FitsException If the argument is not a valid FITS object
*/
private static Object entryToColumnArray(Object o) throws FitsException {
o = boxedToArray(o);

if (o.getClass().isArray()) {
int[] dim = ArrayFuncs.getDimensions(o);

if (dim.length == 1 && dim[0] == 1) {
return o;
}
}

Object[] array = (Object[]) Array.newInstance(o.getClass(), 1);
array[0] = o;
return array;
}

/**
* <p>
* Adds a new column with the specified data array, with some default mappings. This method will always use
Expand Down Expand Up @@ -1908,7 +1883,7 @@ private int checkRowCount(Object o) throws FitsException {
* <code>boolean</code> arrays will stored as logical bytes, otherwise as packed bits.
*/
private int addColumn(Object o, boolean compat) throws FitsException {
o = boxedToArray(o);
o = ArrayFuncs.objectToArray(o, compat);

int rows = checkRowCount(o);

Expand Down Expand Up @@ -2177,30 +2152,62 @@ private int addFlattenedColumn(Object o, int rows, ColumnDesc c, boolean compat)
*/
@Override
public int addRow(Object[] o) throws FitsException {
ensureData();

if (columns.isEmpty()) {
for (Object element : o) {

if (element == null) {
throw new TableException("Prototype row may not contain null");
}

addColumn(entryToColumnArray(element));
Class<?> cl = element.getClass();

if (cl.isArray()) {
if (cl.getComponentType().isPrimitive() && Array.getLength(element) == 1) {
// Primitives of 1 (e.g. short[1]) are wrapped and should be added as is.
addColumn(element);
} else {
// Wrap into array of 1, as leading dimension becomes the number of rows, which must be 1...
Object wrapped = Array.newInstance(element.getClass(), 1);
Array.set(wrapped, 0, element);
addColumn(wrapped);
}
} else {
addColumn(ArrayFuncs.objectToArray(element, true));
}
}
} else if (o.length != columns.size()) {

return 1;
}

if (o.length != columns.size()) {
throw new TableException("Mismatched row size: " + o.length + ", expected " + columns.size());
} else {
Object[] flatRow = new Object[getNCols()];
}

for (int i = 0; i < flatRow.length; i++) {
ColumnDesc c = columns.get(i);
flatRow[i] = c.isVariableSize() ? putOnHeap(c, o[i], null) : javaToFits1D(c, ArrayFuncs.flatten(o[i]));
ensureData();

Object[] flatRow = new Object[getNCols()];

for (int i = 0; i < flatRow.length; i++) {
ColumnDesc c = columns.get(i);
if (c.isVariableSize()) {
flatRow[i] = putOnHeap(c, o[i], null);
} else {
flatRow[i] = javaToFits1D(c, ArrayFuncs.flatten(o[i]));

int nexp = c.getElementCount();
if (c.stringLength > 0) {
nexp *= c.stringLength;
}

if (Array.getLength(flatRow[i]) != nexp) {
throw new IllegalArgumentException("Mismatched element count for column " + i + ": got "
+ Array.getLength(flatRow[i]) + ", expected " + nexp);
}
}
table.addRow(flatRow);
nRow++;
}

table.addRow(flatRow);
nRow++;

return nRow;
}

Expand Down Expand Up @@ -3615,21 +3622,17 @@ protected Object getFromHeap(ColumnDesc c, Object p, boolean isEnhanced) throws
*
* @throws FitsException if the operation failed
*/
private Object javaToFits1D(ColumnDesc c, Object o) throws FitsException {
private static Object javaToFits1D(ColumnDesc c, Object o) throws FitsException {

if (c.isBits()) {
if (o instanceof Boolean && c.isSingleton()) {
// Scalar boxed boolean...
return FitsUtil.bitsToBytes(new boolean[] {(Boolean) o});
}

return FitsUtil.bitsToBytes((boolean[]) o);
}

if (c.isLogical()) {
if (o instanceof Boolean && c.isSingleton()) {
return FitsUtil.booleansToBytes(new Boolean[] {(Boolean) o});
}

// Convert true/false to 'T'/'F', or null to '\0'
return FitsUtil.booleansToBytes(o);
}
Expand Down Expand Up @@ -3676,7 +3679,7 @@ private Object javaToFits1D(ColumnDesc c, Object o) throws FitsException {
return FitsUtil.stringsToByteArray((String[]) o, c.getStringLength(), FitsUtil.BLANK_SPACE);
}

return boxedToArray(o);
return ArrayFuncs.objectToArray(o, !c.isBits);
}

/**
Expand Down Expand Up @@ -3758,45 +3761,6 @@ protected void createTable(int rows) throws FitsException {
nRow = rows;
}

private static Object boxedToArray(Object o) throws FitsException {
if (o.getClass().isArray()) {
return o;
}

// Convert boxed types to primitive arrays of 1.
if (o instanceof Number) {
if (o instanceof Byte) {
return new byte[] {(byte) o};
}
if (o instanceof Short) {
return new short[] {(short) o};
}
if (o instanceof Integer) {
return new int[] {(int) o};
}
if (o instanceof Long) {
return new long[] {(long) o};
}
if (o instanceof Float) {
return new float[] {(float) o};
}
if (o instanceof Double) {
return new double[] {(double) o};
}
throw new FitsException("Unsupported Number type: " + o.getClass());
}

if (o instanceof Boolean) {
return new Boolean[] {(Boolean) o};
}

if (o instanceof Character) {
return new char[] {(char) o};
}

return o;
}

/**
* Sets the input to use for reading (and possibly writing) this table. If the input implements
* {@link ReadWriteAccess}, then it can be used for both reading and (re)writing the data, including editing in
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/nom/tam/fits/FitsUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ public static long addPadding(long size) {
*
* @param o a new array of <code>Boolean</code>
*
* @return and array of FITS logical values with the same size and shape as the input
* @return an array of FITS logical values with the same size and shape as the input
*
* @see #bytesToBooleanObjects(Object)
* @see #byteToBoolean(byte[])
Expand All @@ -126,11 +126,11 @@ public static long addPadding(long size) {
*/
static Object booleansToBytes(Object o) {
if (o == null) {
return FitsEncoder.byteForBoolean(null);
return new byte[] {FitsEncoder.byteForBoolean(null)};
}

if (o instanceof Boolean) {
return FitsEncoder.byteForBoolean((Boolean) o);
return new byte[] {FitsEncoder.byteForBoolean((Boolean) o)};
}

if (o instanceof boolean[]) {
Expand Down
51 changes: 51 additions & 0 deletions src/main/java/nom/tam/util/ArrayFuncs.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1181,4 +1181,55 @@ private static Object sample(Object orig, int[] from, int[] size, int[] step, in
return slice;
}

/**
* Converts objects to arrays. If the object is already an array it is returned unchanged. Boxed primitives are
* returned as primitive arrays of 1. All other objects are wrapped into an array of 1 of the same type.
* <code>Boolean</code> values are somewhat special and are handled according to the second argument, either to
* produce a <code>boolean[1]</code> or else a <code>Boolean[1]</code>.
*
* @param o The object
* @param booleanAsObject Whether <code>Boolean</code> values should be converted <code>Boolean[1]</code> instead of
* <code>boolean[1]</code>.
*
* @return The input object, wrapped into an array as appropriate.
*
* @since 1.21
*/
public static Object objectToArray(Object o, boolean booleanAsObject) {
if (o.getClass().isArray()) {
return o;
}

// Convert boxed types to primitive arrays of 1.
if (o instanceof Number) {
if (o instanceof Byte) {
return new byte[] {(byte) o};
}
if (o instanceof Short) {
return new short[] {(short) o};
}
if (o instanceof Integer) {
return new int[] {(int) o};
}
if (o instanceof Long) {
return new long[] {(long) o};
}
if (o instanceof Float) {
return new float[] {(float) o};
}
if (o instanceof Double) {
return new double[] {(double) o};
}
} else if (o instanceof Boolean) {
return booleanAsObject ? new Boolean[] {(Boolean) o} : new boolean[] {(Boolean) o};
} else if (o instanceof Character) {
return new char[] {(Character) o};
}

Object array = Array.newInstance(o.getClass(), 1);
Array.set(array, 0, o);

return array;
}

}
34 changes: 33 additions & 1 deletion src/test/java/nom/tam/fits/BinaryTableNewTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1207,7 +1207,7 @@ public void testBuildBareRows() throws Exception {
Assert.assertEquals(double.class, tab.getDescriptor(7).getElementClass());
}

@Test(expected = FitsException.class)
@Test
public void testAddColumnNotArray() throws Exception {
BinaryTable tab = new BinaryTable();
tab.addColumn("abc");
Expand Down Expand Up @@ -2081,4 +2081,36 @@ public void testIsNumeric() throws Exception {
Assert.assertFalse(ColumnDesc.createForFixedArrays(boolean.class, 2).isNumeric());
Assert.assertFalse(ColumnDesc.createForFixedArrays(Boolean.class, 2).isNumeric());
}

@Test
public void scalarBitTest() throws Exception {
BinaryTable bt = new BinaryTable();
bt.addColumn(BinaryTable.ColumnDesc.createForScalars(boolean.class));
bt.addRowEntries(true);
// No exception
}

@Test
public void scalarLogicalTest() throws Exception {
BinaryTable bt = new BinaryTable();
bt.addColumn(BinaryTable.ColumnDesc.createForScalars(Boolean.class));
bt.addRowEntries(true);
// No exception
}

@Test(expected = ClassCastException.class)
public void scalarBitTestException() throws Exception {
BinaryTable bt = new BinaryTable();
bt.addColumn(BinaryTable.ColumnDesc.createForFixedArrays(boolean.class, 2));
bt.addRowEntries(true);
// No exception
}

@Test(expected = IllegalArgumentException.class)
public void scalarLogicalTestException() throws Exception {
BinaryTable bt = new BinaryTable();
bt.addColumn(BinaryTable.ColumnDesc.createForFixedArrays(Boolean.class, 2));
bt.addRowEntries(true);
// No exception
}
}
6 changes: 3 additions & 3 deletions src/test/java/nom/tam/fits/FitsUtilTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ public class FitsUtilTest {

@Test
public void singleBooleanToByte() throws Exception {
Assert.assertEquals((byte) 'T', (byte) FitsUtil.booleansToBytes(true));
Assert.assertEquals((byte) 'F', (byte) FitsUtil.booleansToBytes(false));
Assert.assertEquals((byte) 0, (byte) FitsUtil.booleansToBytes(null));
Assert.assertEquals((byte) 'T', ((byte[]) FitsUtil.booleansToBytes(true))[0]);
Assert.assertEquals((byte) 'F', ((byte[]) FitsUtil.booleansToBytes(false))[0]);
Assert.assertEquals((byte) 0, ((byte[]) FitsUtil.booleansToBytes(null))[0]);
}

@Test
Expand Down
35 changes: 2 additions & 33 deletions src/test/java/nom/tam/fits/test/BinaryTableTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,8 @@ public void buildFromEmptyBinaryTable() throws Exception {
strings[0] = "abcdefghijklmnopqrstuvwxyz";

for (int i = 0; i < NROWS; i++) {
tab.addRow(new Object[] {strings[i], shorts[i], floats[i], new double[] {doubles[i]}, multiString[i]});
// tab.addRow(new Object[] {strings[i], shorts[i], floats[i], doubles[i], multiString[i]});
tab.addRow(new Object[] {strings[i], shorts[i], floats[i], doubles[i], multiString[i]});
}

Header hdr = new Header();
Expand Down Expand Up @@ -1772,36 +1773,4 @@ private BinaryTable createTestTable() throws FitsException {
return btab;
}

@Test
public void scalarBitTest() throws Exception {
BinaryTable bt = new BinaryTable();
bt.addColumn(BinaryTable.ColumnDesc.createForScalars(boolean.class));
bt.addRowEntries(true);
// No exception
}

@Test
public void scalarLogicalTest() throws Exception {
BinaryTable bt = new BinaryTable();
bt.addColumn(BinaryTable.ColumnDesc.createForScalars(Boolean.class));
bt.addRowEntries(true);
// No exception
}

@Test(expected = ClassCastException.class)
public void scalarBitTestException() throws Exception {
BinaryTable bt = new BinaryTable();
bt.addColumn(BinaryTable.ColumnDesc.createForFixedArrays(boolean.class, 2));
bt.addRowEntries(true);
// No exception
}

@Test(expected = IllegalArgumentException.class)
public void scalarLogicalTestException() throws Exception {
BinaryTable bt = new BinaryTable();
bt.addColumn(BinaryTable.ColumnDesc.createForFixedArrays(Boolean.class, 2));
bt.addRowEntries(true);
// No exception
}

}
Loading

0 comments on commit b3de704

Please sign in to comment.