Dataset caching

[davidhassell]

See NCAS-CMS/h5netcdf#8 for details

[bnlawrence]

I guess this depends on shape being an integer, so you can remove the redundancy?

[davidhassell]

Just a bit of premature optimisation - math.prod is way faster than np.prod

[bnlawrence]

I think it might be worth putting a much larger comment section in advance of this block explaining the various entities in play. I already can't remember what is going on here and why and a minute of trying to chase it down didn't enlighten me :-).

Tthen, instead of trying if it is an object by trying the method might be better to use the isinstance to check whether it is a particular type of entity that way the code will be much clearer?

[davidhassell]

I see. Would this be a bit better?

            if self.posix:
                fh = open(self._filename, 'rb')
            else:
                try:
                    # Try s3 being an s3fs.S3File object
                    fh = fh.s3.open(self._filename)
               except AttributeError:
                    raise SomeSortOfError("unknown file object type")

[bnlawrence]

That's better, but is there a reason why we wouldn't do:

if self.posix:
    fh=open(self._filename,'rb')
elif isinstance(fh,s3fs.S3File):
   fh=fh.s3.open(self._filename)
else:
   raise SomeSortofError(...)

?

[davidhassell]

I suppose it boils down to if we want to "Look Before You Leap", or else "Easier to Ask Forgiveness". I'm fine with either approach (assuming that there are no other libraries that share the s3fs API) and the former (i.e. the code you suggested) is certainly more explicit.

[bnlawrence]

Thanks. If you're ok with both, I prefer the former, as it's clearer to (this) reader :-).