Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

read_spe() arguments acc2avg and cts_sec #37

Open
cbeleites opened this issue Aug 10, 2021 · 4 comments
Open

read_spe() arguments acc2avg and cts_sec #37

cbeleites opened this issue Aug 10, 2021 · 4 comments

Comments

@cbeleites
Copy link
Contributor

read_spe() differs from all other file import functions in that it offers two more options for post-processing:

  • acc2avg which triggers averaging accumulations (instead of the summation done by the Winspec software)
    i.e. dividing the spectral intensities by the number of accumulations, and
  • cts_sec which will divide the spectral intensities by the exposure time

Do we want to keep this functionality or do we prefer that any such non-standard treatment should be done by the user afterwards?

@GegznaV
Copy link
Member

GegznaV commented Aug 10, 2021

If these are not "standard" procedures for this file format, then pre-processing should be separated from data import functionality.

@cbeleites
Copy link
Contributor Author

@bryanhanson wrote:

I say let the user modify their data if they want, don't do it automatically, and make this clear in the documentation.

@cbeleites
Copy link
Contributor Author

@GegznaV I don't remember sufficently well what take Winspec has on this question, but you encounter all 4 possibilities across different spectrometer software.

However, it is not easy for us do offer this pre-processing across different file formats because there is no standard how the parameter/key/colum storing the exposure time and number of accumulations is called. There isn't even a standard whether exposure time is total exposure time or exposure time per accumulation.

For the user, doing this themselves will be 2 lines of code: one for dividing the spectral intensities by the respective column and one for updating the labels() of the intensity axis.

@ximeg
Copy link

ximeg commented Aug 30, 2021

I'm in favor of removing these non-standard arguments. Let's keep all import functions lean and uniform.

As far as I remember, I added these arguments many years ago when I was implementing this function. Back then I was the only user of this function, and I wanted it to do all of these things for me - I felt lazy to add two lines of code for pre-preprocessing afterward :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants