das-developers / das2dlm

IDL binary extension das2 client
GNU Lesser General Public License v2.1
2 stars 0 forks source link

The !NULL problem #6

Closed cpiker closed 4 years ago

cpiker commented 4 years ago

das2dlm functions return !null when some action couldn't complete so that higher level programs could gracefully handle problems.

Unfortunately, the !null system variable was not introduced into IDL until version 8.0. For compatibility with old versions of IDL, the API will have to throw messages or return something other then !null for invalid requests.

xandrd commented 4 years ago

I think that a potential data set could have some !NULL value data points. As I understand, throwing a message is equivalent to an error (e.g. no data set will be returned), which is inconvenient. If we need to replace the value, then some sort of "FILLVAL" should be defined for each data type. This will result in a lot of specific data checking while using the library. Let me make sure first that supporting IDL bellow 8.0 is absolutely necessary.

cpiker commented 4 years ago

So NaN is often used for fill values. But !NULL can't be used as fill. The das2dlm uses !null so that calls like:

dim_time = das2c_pdims(dataset, 'time')

can return !null in instances where the named dimension is not in the dataset instead of throwing an error message. Either way there's no "time" data to get, it's just a flow control issue.

The issue at hand is, should the caller find out there is no time dimension in the dataset by getting a !null or by checking that values exist prior to the call, or by catching an exception. It seemed to me that it's easier to write application code that checks for !null then it is to write code that looks through a dimension list for the desired item, or to deal with message handlers.

With !null returns allowed, calling code can look like this:

pd_time = das2c_pdims(dataset, 'time')
if pd_time eq !null then print, 'dataset has no time dimension!'

Without !null return, the caller has to check that dim exists manually, or an exception will be thrown:

all_dims = das2c_pdims(dataset)
has_time = !false
for i = 0, n_elements(all_dims) - 1 do begin
   if all_dims[i].pdim eq 'time' then begin
      has_time = !true
   endif
endfor
if ~has_time then print, 'dataset has no time dimension!'

I think that !null returns are cleaner, but the API could be refactored to do without it and I'm content to change the API if spedas needs to support a pre-8.0 version of IDL.

I look forward to hearing your preference.

xandrd commented 4 years ago

I agree that using !null is preferable. I will double check with SPEDAS team and let you know their opinion as well.

xandrd commented 4 years ago

After discussion with SPEDAS team we decided that support IDL >= 8.0 is sufficient. das2dlm is a new feature and there is no need to support legacy features. SPEDAS is migrating to minum version of 8.4 and only few components are still compatible with older IDL version. Hence, we can move forward with !null variable.

cpiker commented 4 years ago

Documentation updates to indicate the minimum IDL version number closes this issue.