Allow `custom_field_map` to override existing field types for thread safety

[eleftherioszisis]

Retrieving the field type in read_header and _write_header requires to access a global variable.

This introduces two serious issues:

1. Using in the same code pynrrd in combination with another library that changes the globals, e.g. SPACE_DIRECTIONS_TYPE, may result in unexpected behavior because importing the latter would affect how the former works.
2. Thread safety

While 1. can be solved with a context manager that changes the globals for the read/write operations and then restores them, 2. cannot be guaranteed when many threads needs to change and restore that global.

I would strongly suggest to avoid using globals in pynrrd, however I understand that changing the public API is difficult and takes time. For this reason I would like to suggest a workaround that would require to adapt how the custom_field_map is used.

In:

pynrrd/nrrd/reader.py

Line 90 in fa37782

def _get_field_type(field: str, custom_field_map: Optional[NRRDFieldMap]) -> NRRDFieldType:

the custom_field_map is used only for extra/unknown fields. However, this could be adapted to allow overrides if those are specified in the custom_field_map. This way the:

    elif field in ['space directions']:
        return nrrd.SPACE_DIRECTIONS_TYPE

would become:

    elif field in ['space directions']:
        return custom_field_map.get("space directions", nrrd.SPACE_DIRECTIONS_TYPE)

And ideally nrrd.SPACE_DIRECTIONS_TYPE could become a default value instead of a global that the user can change.

[addisonElliott]

I see and understand your points. My mindset behind this global variable is that it's a temporary stopgap in order to gracefully handle the breaking change of the space directions type. In v1.x, the default value will stay as double matrix but v2.x will transition the default to double vector list.

This global variable lets you get a sneak peek of the upcoming default value. If you import the library, change the global variable and leave it alone after that, then thread-safety is irrelevant.

To me, I don't see how this is any different than if v2.x introduced this breaking change and a library uses v2.x, then any projects depending on the library will be incompatible with v1.x. My point being is that I don't think the current setup is any less flexible.

For this reason I would like to suggest a workaround that would require to adapt how the custom_field_map is used.

Open to this change. I do agree it maximizes flexibility in terms of compatibility.

I still believe the global variable solution is fine in this situation. I prefer this approach over the custom_field_map because you don't have to remember to include this in each nrrd.read()/nrrd.write() call.