-
Notifications
You must be signed in to change notification settings - Fork 20
[Don't merge] Octave-compatible changes #2
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
base: dev
Are you sure you want to change the base?
Conversation
I now added the Python plot script and an example CSV data file. The data was generated with your code, namely data = ImportAgilent()
% Then I chose the .CH file in the file choser
ExportCSV(data.time, data.intensity, 'file', 'hplc-out.csv')
% Please note that I used data.time and data.intensity, regarding your last message.
% So it should already be corrected (corrected_signal = raw_signal * slope + intercept).
% Data.time was returned in [min], but as you later corrected me, it is expected to be
% in [s] for the peak area calculation. Therefore there's an `* 60` in the Python plot script However, the file name last parameter did not work correctly. the file was always named data.CSV. I am not entirely sure, but I think I accidentally downloaded Greetings, PS.: To execute the Python script, use a Terminal/CMD (depending on your OS) and go into the directory where the .CSV example file is located. Then run |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you're interested, here are the locations where I used the information you gave me in your last message.
print('Baseline is', baseline) | ||
|
||
# Apply baseline correction | ||
peak_y_vals = [y_val - baseline for y_val in peak_y_vals] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is where I subtract the baseline from each y-value
|
||
SIGNAL = y_vals[i] | ||
|
||
if baseline > SIGNAL: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here I search for the minimum y-value for the baseline
# Iterate over each line and populate list | ||
for row in reader: | ||
# Read data and parse to float | ||
TIME = float(row[0]) * 60 # Time val is given as minute (float), hence convert to seconds |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here I convert the minutes to seconds
for y_val in y_vals: | ||
i = i + 1 | ||
|
||
if x_vals[i] < PEAK_START_TIME or x_vals[i] > PEAK_END_TIME: # Skip if time is not in between PEAK_START_TIME and PEAK_END_TIME |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here I create a subset of the original data, that is, the peak itself limited by PEAK_START_TIME
and PEAK_END_TIME
. As you already said: This is subjective, but it is precise enough. The constants are defined at the beginning of the script. Please note that the values will be refined later, it was just an approximation to validate the calculation.
Hello James,
as per your request, here are the changes I made to make your
ImportAgilent.m
code compatible with Octave 4.0.1 (as it is shipped with Debian 9.2). I thought that agit diff
would be a much better way to show the changes, that's why I made a pull request. I don't expect you to merge these changes, especially since they break theinputParser
, but it is much more comprehensible that way.I also added comments explaining the reasons why I had to made that particular change, but for simplicity's sake, here is a summary:
addParameter
(method ofinputParser
) does not exist in Octave (Version 4.0.1). Here is the corresponding issue. It looks like they have some patches available, but I was not sure about that. Also, I did not need the functionality and it was easier for me to just short-circuit it. I don't know if there's an alternative for thatjavaObject('fully.qualified.class.name', args...)
to initialise a Java object. It seems like this functionality is also available in MATLAB - at least I found a source/documentation where they explicitly mention MATLABcom.mathworks.*
obviously doesn't exist in Octave, but the functionality you want is implemented in the native Java libraries, namely Swing. Initially I just dropped the filter code, because I just picked the .CH files and that's it, but I really think that this is more elegant since it uses native libraries instead of some MATLAB façade.Now that I am writing this PR I see that I didn't properly change line 349:
To exactly replicate the behaviour, you'd need to do it like that: (may not work, that's from the top of my head)
I am aware of the fact that the
javaObject
syntax is not as pretty as writing the fully qualified class name like it would be Java-native, but at least it works in Octave as well.Please let me know if there's anything I can help you with.
Greetings,
Chris