issue_comments: 381467470

OK, I pushed a couple of small changes to your branch. Generally this is looking pretty good. I have a couple of other minor comments that I will post inline.

Your issues with the skipped tests (which I switched to xfail) are part of a large issue in xarray, which is that we don't have good ways to handle backend specific decoding (https://github.com/pydata/xarray/issues/2061).

I agree that we can probably put this most of them for now (especially the string encoding), but I'm somewhat concerned about how decoding could end up with reading incorrect data values, e.g., if a source using scale/offset encoding. Consider this failed test: ``` def test_roundtrip_mask_and_scale(self): decoded = create_masked_and_scaled_data() encoded = create_encoded_masked_and_scaled_data() with self.roundtrip(decoded) as actual:

      assert_allclose(decoded, actual, decode_bytes=False)

E AssertionError: [ nan nan 10. 10.1 10.2] E [ nan nan 11. 11.01 11.02] ```

These sort of bugs can be pretty insidious, so if there's any chance that someone would use PNC to read a netCDF file with this sort of encoding we should try to fix this before merging this in.

One simple approach would be to raise an error for now if mask_and_scale=True in open_dataset(), that is, to force the user to explicitly disable masking and scaling with xr.open_dataset(filename, engine='pseudonetcdf', mask_and_scale=False).

Alternatively, I suppose we could switch the default value to mask_and_scale=None, and pick True or False based on the choice of backend.