Add proper module ID (no board)
    Add methods as needed
    cobraId vs. scienceFiberId? Why two, really?
    fiducials? Separate class, probably.
    Identify engineering/blank holes?

While you are working on this, can you file a ticket for each of these and include their numbers in this list? We shouldn't have any TODOs that aren't associated with a jira ticket.

[ That is from an old comment in fiberids.py, I believe. Looks familiar. Did generate requests on some if the recent GFM tickets. Comment should be removed/updated. ]

From the INSTRM-1195, etc. work:

• module ID added
• cobraId/fiberHoleId/scienceFiberId/fiberId better understood and made more sane.
• eng/blank rows added.

That leaves the fiducial fibers and the "other methods". INSTRM-1259 addresses the "other methods".

The fiducial fibers are described in another pfs_utils/data/fiberids/ data file, which is read in and used directly by some part of the per-exposure/per-field fps/mcs calibration scheme. I do not know enough about that to say whether/how that API should be formalized.

Main problem is that the reading in of the SuNSS fiberId was incorrect due to the wrong datatype for that column being specified. That has been fixed and a regression test added to trap situations in the future where changes are made to the data or corresponding fiber-related utilities.

Changes are captured in https://github.com/Subaru-PFS/pfs_utils/pull/29

See GH. I'm not sure that I like the masked array choice, and there's at least one typo.

As of this morning the values of fiberIds.sunssFiberId do not correspond to the values in the file. Please change this back; I'd like to see "emp" or None not --. The same goes for other missing fields, please either return the value in the file or None/NaN and remove the use of masked arrays.

In the latest push to the INSTRM-1258 I've removed the use of MaskedArrays. But a problem remains in the handling of columns of integer type. As far as I can see, it is not possible to assign a value of None or np.nan. A legal integer value as the fill value is of course fine to do, and is currently 65535. If None or np.nan really required I need technical advice. price?

Following slack discussion, a fill value of 65535 for unsigned integer fields is acceptable.

If it's not too late, can we rename MISSING_VALUE_UINT16 to e.g. MISSING_VALUE? The fact that that's a U16 number is irrelevant.

Not too late, and one of Paul's comments. Ok for MISSING_VALUE_INTEGER? If so, already pushed.

That's OK. The code will look like:

if fieldId == MISSING_VALUE_INTEGER:
    continue

and I personally think that the _INTEGER detracts from the readability as the type is irrelevant, but go with what you prefer.

I would also prefer to remove _INTEGER, but problem is that this value is only used for the integer types. The float types use np.nan. The string types propagate - or emp.

I've gone with rhl's suggestion and renamed to MISSING_VALUE. It's probably the most optimal name.

Merged to master.