xdf-modules / pyxdf

Python package for working with XDF files
BSD 2-Clause "Simplified" License
34 stars 16 forks source link

We shouldn't be rounding nominal_srate in load_xdf #67

Closed cboulay closed 4 years ago

cboulay commented 4 years ago

I've seen devices that advertise non-integer sampling rates. TDT comes to mind, though a quick scan of their online docs give numbers like "~50kHz". Also, I have a couple apps where the user can specify the sampling rate in float. There's no good reason to limit the sampling rate to integer.

cbrnr commented 4 years ago

No objection, but I would like to make sure we display sensible values. If a device has a ~50kHz sampling rate, I don't think it makes a lot of sense to be super precise and add decimal places. In other cases, one or two decimal places might make sense (e.g. 16.25Hz). I would like to avoid displaying non-significant places (e.g. 16.250000000000Hz), so we shouldn't use a fixed-width format. So maybe rounding to at most two decimal places would be a viable compromise (so we'd have e.g. 512Hz and not 512.00Hz if the rate is integer).

cboulay commented 4 years ago

What specifically are you referring to by "we display"? We're just loading data into memory. How the numbers get printed on screen isn't a problem for load_xdf. Maybe we need a pretty_print function but that's a separate issue.

I'm more concerned with rounding the sampling rate when it shouldn't be rounded, and then using that number in other places. e.g. incorrectly calculating timestamps: self.tdiff = 1.0 / self.srate if self.srate > 0 else 0.0 --> stamps[k] = s.last_timestamp + s.tdiff

cbrnr commented 4 years ago

True, we can take care of formatting the number when displaying it.

cbrnr commented 4 years ago

What was bothering me with the example file is that it contained numbers with 15 fixed decimal places in its XML meta information. We should advise people to use the minimum number of decimal places.

cboulay commented 4 years ago

That's a LabRecorder issue. I just looked through its source. It seems to be using a one-size-fits-all function for writing the header. I don't know if we want to add in some extra code to truncate trailing zeros.

cbrnr commented 4 years ago

I don't know if we want to add in some extra code to truncate trailing zeros.

I think that would be a good idea.