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

Options to select data to plot #15

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ondrejtichacek
Copy link

Added parameter -w --wavelength to select data to plot. Plots all data if left empty. Multiple data rows can be selected as e.g. -w 215nm -w 280nm.

Added parameter -w --wavelength to select data to plot. Plots all data if left empty. Multiple data rows can be selected as e.g. -w 215nm -w 280nm.
@pyahmed
Copy link
Owner

pyahmed commented Jul 21, 2017

Wouldn't it be better to say -w1 -w2 -w3 etc? This way one does not need to know the wavelength that was actually used?

@Guillawme
Copy link

I agree it would be easier if the user doesn't have to know (or remember) the wavelength in advance.

Also, it should handle corner cases like the user asking for -w3 while no wavelength 3 was recorded in this dataset.

@ondrejtichacek
Copy link
Author

I think if the users don't remember the exact wavelengths, they would usually plot all wavelengths first and then see which they like. Determining the data row by the wavelength is also very precise. Therefore I would like to keep this option.

I agree, the alternative option may be useful for someone, however, I see the following problems:

  • I don't know if there is a way how to define an arbitrary number of parameters -w1, -w2, -w3, ...
  • The datasets are not ordered by wavelengths, which can be misleading
  • The datasets may be ordered differently at different systems. For example, for data coming from different machines having different detectors, the option by exactly specifying the wavelength works better.

@pyahmed
Copy link
Owner

pyahmed commented Jul 21, 2017

It would work like this:
-w1 True/False [Default: True]
-w2 True/False [Default: True if present]
-w3 True/False [Default: False]

Judging by the systems/files that I have come across:

  • ÄKTA Prime / Prime Plus:
    always only have a single wavelength, which is manually set to 260 or 280. Within the res file this is just labeled "UV"

  • ÄKTA Purifiers:
    Depending on UV detector, either single or three, labeled labeled UV1_###nm, UV2_###nm and UV3_###nm".

  • ÄKTA Pure:
    Same as Purifiers as far as I can remember.

The reason I prefer the -w1/2/3 is that the command line does not change because of the wavelength, which is especially helpful when integrating pycorn-bin in a script. I think w1/2/3 would be sufficient, since I think there are no systems which have more than 3 wavelengths.

@ondrejtichacek
Copy link
Author

ondrejtichacek commented Jul 22, 2017

I have access to two ÄKTA Purifiers, having three UV detectors, 215, 254, and 280 nm. The result files are, however, not entirely equal - the wavelengths are ordered differently: 280, 254, 215 on the first machine, 280, 215, 254 on the second one. Therefore, using for example -w1 -w2 would not yield the same result, but using -w 280nm -w 254nm would. I would find it very annoying to have to have two different scripts for plotting my data based on where I measured them.

Also, I think, by default, the script should plot everything - it might otherwise lead to confusion of new users.

Add parameter -o --output to specify output file name (default still works)
@pyahmed
Copy link
Owner

pyahmed commented Jul 22, 2017

Regarding ordering etc.. do the data series have a prefix like UV1_, UV2_ etc?

Current behaviour is to plot everthing unless it is set to 0nm ("off").

pyahmed
pyahmed previously approved these changes Jul 22, 2017
@pyahmed
Copy link
Owner

pyahmed commented Jul 22, 2017

Could you send the output file naming patch separately?

@pyahmed pyahmed dismissed their stale review July 22, 2017 15:44

split up in 2 patches

@ondrejtichacek
Copy link
Author

The data series from both machines have the same names - UV1_280nm, etc.

The latest commit is reverted - sorry, I didn't realise that it will affect this pull request.

@pyahmed
Copy link
Owner

pyahmed commented Jul 25, 2017

Could you make a separte pull request with the output file naming only? Since that one can be merged easily.

@ondrejtichacek
Copy link
Author

Yes, I'll do it as soon as possible. I'd like to first clean it up and possibly add more functionality regarding the output file names.

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

Successfully merging this pull request may close these issues.

None yet

3 participants