Uploaded image for project: 'LIMS'
  1. LIMS
  2. LIMS-1805

Instrument Interface. Horiba JY ICP

    Details

    • Type: New Feature
    • Status: Closed
    • Priority: Critical
    • Resolution: Duplicate
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: None
    • Labels:
    • Epic Link:
    • Sprint:
      May 2015, Aug 2015

      Description

      I asked for the instruments full name and update the issue title then. Please ignore the XLS and use a CSV version of it

      1. ICP limstest bog envas.csv
        32 kB
        Pau Soliva
      2. lemoene 150806.txt
        2 kB
        lemoene
      3. RAW ICP 2013-12.pdf
        79 kB
        lemoene
      4. RAW ICP 2013-12.xls
        96 kB
        lemoene
      1. icpxmpl.png
        22 kB
      2. jira-capture-screenshot-20150922-131433-673.png
        171 kB
      3. jira-capture-screenshot-20150922-131551-859.png
        297 kB
      4. jira-capture-screenshot-20150922-134239-819.png
        174 kB

        Issue Links

          Activity

          Hide
          Espurna Pau Soliva (Inactive) added a comment -

          dlemoene, do you know the instrument name? It's not critical to write the importer, but will be useful for code organization and folders naming.

          Show
          Espurna Pau Soliva (Inactive) added a comment - d lemoene , do you know the instrument name? It's not critical to write the importer, but will be useful for code organization and folders naming.
          Hide
          lemoene lemoene added a comment - - edited

          from email comms, in example line for Ca:

          > The parameter tested for is Ca. 315.887 is the line that Ca is tested at. Ca can be tested at different lines. eg. Ca 315, Ca 250 etc.
          ah, wave lengths in nm. You want to report it just as Calcium
          > So when you read the parameter you should only read the first two characters: Ca and not Ca 315.
          It does no seem separated into 2 fields in the file though... Will it always be 2 character periodic table elements?

          Coumn F holds the result Concentration

          > In the attached screenshot example, the 1st sample QC 350 PPM's Ca 315.887 Concentration test result is 332.779 mg/l

          > What is 4.99 in column G? And 1.5 in J?
          > f= Concentration
          > g = Standard deviation
          > j = residual error

          I think we load them all Pau, including the line wavelenth. We do not have to display them but then they are available when required. No extra programming required.

          Show
          lemoene lemoene added a comment - - edited from email comms, in example line for Ca: > The parameter tested for is Ca. 315.887 is the line that Ca is tested at. Ca can be tested at different lines. eg. Ca 315, Ca 250 etc. ah, wave lengths in nm. You want to report it just as Calcium > So when you read the parameter you should only read the first two characters: Ca and not Ca 315. It does no seem separated into 2 fields in the file though... Will it always be 2 character periodic table elements? Coumn F holds the result Concentration > In the attached screenshot example, the 1st sample QC 350 PPM's Ca 315.887 Concentration test result is 332.779 mg/l > What is 4.99 in column G? And 1.5 in J? > f= Concentration > g = Standard deviation > j = residual error I think we load them all Pau, including the line wavelenth. We do not have to display them but then they are available when required. No extra programming required.
          Hide
          johan Johan Fourie added a comment - - edited

          > So when you read the parameter you should only read the first two characters: Ca and not Ca 315.
          It does no seem separated into 2 fields in the file though... Will it always be 2 character periodic table elements?

          Not always. Some elements like potassium (K).

          Maybe if one can read the parameter then until you get to the space. Dont know if that is possible.

          Show
          johan Johan Fourie added a comment - - edited > So when you read the parameter you should only read the first two characters: Ca and not Ca 315. It does no seem separated into 2 fields in the file though... Will it always be 2 character periodic table elements? Not always. Some elements like potassium (K). Maybe if one can read the parameter then until you get to the space. Dont know if that is possible.
          Hide
          lemoene lemoene added a comment -

          From email

          Use Case. Importing Re-tests

          • We measure a sample BOG311 on an instrument
          • We are not happy with the results and do a re-test
          • These results appear on the same file ...
          • Only use the latest instance of sample QC 350 PPM in column B

          > What would you recommend?

          If i had a pocket full of dollahs, import them all and select in the LIMS which one to publish. Bika keeps original values retested but don't show them to the Client.

          Jordi, Pau. How many extra hours for this easier way:

          • Import values sequentially
          • If a later result for a test is read, imediately save the old one as Re-tested
          • Repeat till the final is loaded

          This is imo the most correct. I don't think we can get away without it.

          Show
          lemoene lemoene added a comment - From email Use Case. Importing Re-tests We measure a sample BOG311 on an instrument We are not happy with the results and do a re-test These results appear on the same file ... Only use the latest instance of sample QC 350 PPM in column B > What would you recommend? If i had a pocket full of dollahs, import them all and select in the LIMS which one to publish. Bika keeps original values retested but don't show them to the Client. Jordi, Pau. How many extra hours for this easier way: Import values sequentially If a later result for a test is read, imediately save the old one as Re-tested Repeat till the final is loaded This is imo the most correct. I don't think we can get away without it.
          Hide
          lemoene lemoene added a comment -

          Maybe if one can read the parameter then until you get to the space.

          Indeed

          Show
          lemoene lemoene added a comment - Maybe if one can read the parameter then until you get to the space. Indeed
          Hide
          johan Johan Fourie added a comment -

          sounds nice if you could do it like that

          Show
          johan Johan Fourie added a comment - sounds nice if you could do it like that
          Show
          Espurna Pau Soliva (Inactive) added a comment - https://github.com/bikalabs/Bika-LIMS/pull/1513
          Hide
          xispa Jordi (Inactive) added a comment -

          lemoene's approach looks good to me.

          ---8<-----------------------
          Not for this case (in which two different files of results are used sequentially), but there was a discussion somewhere regarding to when more than one sample for the same analysis is found inside the same file. In that case, the importer only takes into account the latest result and I don't think creating and retracting analyses found previously would be correct: for example, you could find a results file (If I remember correctly, I saw something like this some months ago) where the first result is the measurement 1, the second, the measurement 2 ... and so on whilst the last result is the average from the previous values. Obviously, by default we only import the last result reported, but there's also the option to import the rest of measurements as interims.
          --->8----------------------

          Show
          xispa Jordi (Inactive) added a comment - lemoene 's approach looks good to me. --- 8< ----------------------- Not for this case (in which two different files of results are used sequentially), but there was a discussion somewhere regarding to when more than one sample for the same analysis is found inside the same file. In that case, the importer only takes into account the latest result and I don't think creating and retracting analyses found previously would be correct: for example, you could find a results file (If I remember correctly, I saw something like this some months ago) where the first result is the measurement 1, the second, the measurement 2 ... and so on whilst the last result is the average from the previous values. Obviously, by default we only import the last result reported, but there's also the option to import the rest of measurements as interims. --- >8 ----------------------
          Hide
          lemoene lemoene added a comment - - edited

          Thanks for the quote Jordi. I was actually thinking that we should apply this for all instruments centrally, but forgot about averaging, ideally thus Bika should be flexible and include choices for these,

          • average duplicates - load duplicates as interims, or
          • use last result and all load all earlier ones as retracted re-tests, or
          • use last result and disregard all other

          Please comment and improve, we can bend Johan's arm on the basis of his minimum requirements to contribute to doing it properly

          Show
          lemoene lemoene added a comment - - edited Thanks for the quote Jordi. I was actually thinking that we should apply this for all instruments centrally, but forgot about averaging, ideally thus Bika should be flexible and include choices for these, average duplicates - load duplicates as interims, or use last result and all load all earlier ones as retracted re-tests, or use last result and disregard all other Please comment and improve, we can bend Johan's arm on the basis of his minimum requirements to contribute to doing it properly
          Hide
          lemoene lemoene added a comment -

          errors on site. more soon

          Show
          lemoene lemoene added a comment - errors on site. more soon
          Hide
          lemoene lemoene added a comment -

          This is the file that is giving us trouble, possibly because of the CR LF at the end of each line

          Show
          lemoene lemoene added a comment - This is the file that is giving us trouble, possibly because of the CR LF at the end of each line
          Hide
          Espurna Pau Soliva (Inactive) added a comment - - edited

          lemoene I used this csv to develop the interfice -> ICP limstest bog envas.csv

          Show
          Espurna Pau Soliva (Inactive) added a comment - - edited lemoene I used this csv to develop the interfice -> ICP limstest bog envas.csv
          Hide
          lemoene lemoene added a comment -

          trace back:

          Traceback (most recent call last):
          File "/usr/local/plone-4.3/metrontest/src/bika.lims/bika/lims/exportimport/instruments/horiba/jobinyvon/icp.py", line 71, in Import
          importer.process()
          File "/usr/local/plone-4.3/metrontest/src/bika.lims/bika/lims/exportimport/instruments/resultsimport.py", line 315, in process
          self._parser.parse()
          File "/usr/local/plone-4.3/metrontest/src/bika.lims/bika/lims/exportimport/instruments/resultsimport.py", line 216, in parse
          jump = self._parseline(line)
          File "/usr/local/plone-4.3/metrontest/src/bika.lims/bika/lims/exportimport/instruments/horiba/jobinyvon/_init_.py", line 23, in _parseline
          line_before, net_intensity, line_after = line.split('"')
          ValueError: too many values to unpack

          Show
          lemoene lemoene added a comment - trace back: Traceback (most recent call last): File "/usr/local/plone-4.3/metrontest/src/bika.lims/bika/lims/exportimport/instruments/horiba/jobinyvon/icp.py", line 71, in Import importer.process() File "/usr/local/plone-4.3/metrontest/src/bika.lims/bika/lims/exportimport/instruments/resultsimport.py", line 315, in process self._parser.parse() File "/usr/local/plone-4.3/metrontest/src/bika.lims/bika/lims/exportimport/instruments/resultsimport.py", line 216, in parse jump = self._parseline(line) File "/usr/local/plone-4.3/metrontest/src/bika.lims/bika/lims/exportimport/instruments/horiba/jobinyvon/_ init _.py", line 23, in _parseline line_before, net_intensity, line_after = line.split('"') ValueError: too many values to unpack
          Hide
          lemoene lemoene added a comment -

          we hacked an existing XLS and could've screwed the format and layout too. Or saved it with the wrong encoding... investigating

          Show
          lemoene lemoene added a comment - we hacked an existing XLS and could've screwed the format and layout too. Or saved it with the wrong encoding... investigating
          Hide
          lemoene lemoene added a comment -

          See https://jira.bikalabs.com/browse/LIMS-1988 re dealing with multiple instruments using the same keywords.

          To get pass it at Metron, we can use the full LineName from the file, e.g. Ca 315.887 but strip the space and . out of it as keywords do not allowed that in the LIMS. 315.887 refers to the wavelength in nm of the Ca's spectral analysis.

          e.g. read Ca 315.887 in the file and look up Ca315887 in the system, Mg 279.806 for Mg279806 etc. The element abbreviation and wavelength combination is pretty unique the lab ensures me.

          Please quote for this customisation. No doubt we'll run into this eslewhere too where the line name cannot be used and then we can reconsider the deluxe solution at https://jira.bikalabs.com/browse/LIMS-1988.

          More from IRC:

          <lemoene> rockfruit, do you know whether AS keywords will handle spaces and full stops, e.g 'Ca 315.887' ?
          <rockfruit> spaces? I really want to say no to spaces. let me check.
          <lemoene> maybe we could just strip them out. NP
          <lemoene> full stops?
          <lemoene> Ca 315.887 is the wavelength the ICP test for and appears like that in the results file
          <rockfruit> accepts [A-Za-z\w\d\-\_] - that's a-z, A-Z, word-characters, numbers, dashes and underscores. anything else fails validation
          <lemoene> just strip them out before loading and keep a keyword Ca315887 in the LIMS?
          <rockfruit> I'm not sure why the validator is so restrictive. offhand, can't think of a reason we couldn't allow keywords to be anything they want to be. BUT I'm sure we had a reason.... so needs some exploratory digging.
          <lemoene> if stripping them out works, fine
          <rockfruit> We use it for the ID of analyses in the AR... there are some restrictions there..
          <lemoene> ok. not lose time
          <rockfruit> yeah stripping sounds good, and is also a good plan. stripped, then the same no-duplications check applied. cannot clash with another service keyword, and cannot clash with a calculation interim field's ID.

          Show
          lemoene lemoene added a comment - See https://jira.bikalabs.com/browse/LIMS-1988 re dealing with multiple instruments using the same keywords. To get pass it at Metron, we can use the full LineName from the file, e.g. Ca 315.887 but strip the space and . out of it as keywords do not allowed that in the LIMS. 315.887 refers to the wavelength in nm of the Ca's spectral analysis. e.g. read Ca 315.887 in the file and look up Ca315887 in the system, Mg 279.806 for Mg279806 etc. The element abbreviation and wavelength combination is pretty unique the lab ensures me. Please quote for this customisation. No doubt we'll run into this eslewhere too where the line name cannot be used and then we can reconsider the deluxe solution at https://jira.bikalabs.com/browse/LIMS-1988 . More from IRC: <lemoene> rockfruit, do you know whether AS keywords will handle spaces and full stops, e.g 'Ca 315.887' ? <rockfruit> spaces? I really want to say no to spaces. let me check. <lemoene> maybe we could just strip them out. NP <lemoene> full stops? <lemoene> Ca 315.887 is the wavelength the ICP test for and appears like that in the results file <rockfruit> accepts [A-Za-z\w\d\-\_] - that's a-z, A-Z, word-characters, numbers, dashes and underscores. anything else fails validation <lemoene> just strip them out before loading and keep a keyword Ca315887 in the LIMS? <rockfruit> I'm not sure why the validator is so restrictive. offhand, can't think of a reason we couldn't allow keywords to be anything they want to be. BUT I'm sure we had a reason.... so needs some exploratory digging. <lemoene> if stripping them out works, fine <rockfruit> We use it for the ID of analyses in the AR... there are some restrictions there.. <lemoene> ok. not lose time <rockfruit> yeah stripping sounds good, and is also a good plan. stripped, then the same no-duplications check applied. cannot clash with another service keyword, and cannot clash with a calculation interim field's ID.
          Hide
          lemoene lemoene added a comment - - edited

          Some imported. Will investigate the others.

          I think the only bug could be the error message that does not rseolve

          And implementing the fix to use the full Linename as keyword.

          Investigatig. Uploading file ICPTest20160922.csv i ran this test with. Worksheet metrontest.bikalabs.com/worksheets/WS15-002

          Show
          lemoene lemoene added a comment - - edited Some imported. Will investigate the others. I think the only bug could be the error message that does not rseolve And implementing the fix to use the full Linename as keyword. Investigatig. Uploading file ICPTest20160922.csv i ran this test with. Worksheet metrontest.bikalabs.com/worksheets/WS15-002
          Hide
          lemoene lemoene added a comment -

          Confirmed. Imports correctly. It is only the Error message ${analysis_keyword) that needs to be resolved. The 'error message'itself is correct - the file includes AS Keywords for Analyses not requested

          Show
          lemoene lemoene added a comment - Confirmed. Imports correctly. It is only the Error message ${analysis_keyword) that needs to be resolved. The 'error message'itself is correct - the file includes AS Keywords for Analyses not requested
          Hide
          lemoene lemoene added a comment - - edited

          So this is the only bit that remains. needs a quote, cannot be too difficult. Lower priority for now

          • Resolve ${analysis_keyword) in error message
          • Use full linename (quote) as keyword

          See LIMS-1988 re dealing with multiple instruments using the same keywords.

          To get pass it at Metron, we can use the full LineName from the file, e.g. Ca 315.887 but strip the space and . out of it as keywords do not allowed that in the LIMS. 315.887 refers to the wavelength in nm of the Ca's spectral analysis.

          e.g. read Ca 315.887 in the file and look up Ca315887 in the system, Mg 279.806 for Mg279806 etc. The element abbreviation and wavelength combination is pretty unique the lab ensures me.

          Please quote for this customisation. No doubt we'll run into this eslewhere too where the line name cannot be used and then we can reconsider the deluxe solution at LIMS-1988.

          Show
          lemoene lemoene added a comment - - edited So this is the only bit that remains. needs a quote, cannot be too difficult. Lower priority for now Resolve ${analysis_keyword) in error message Use full linename (quote) as keyword See LIMS-1988 re dealing with multiple instruments using the same keywords. To get pass it at Metron, we can use the full LineName from the file, e.g. Ca 315.887 but strip the space and . out of it as keywords do not allowed that in the LIMS. 315.887 refers to the wavelength in nm of the Ca's spectral analysis. e.g. read Ca 315.887 in the file and look up Ca315887 in the system, Mg 279.806 for Mg279806 etc. The element abbreviation and wavelength combination is pretty unique the lab ensures me. Please quote for this customisation. No doubt we'll run into this eslewhere too where the line name cannot be used and then we can reconsider the deluxe solution at LIMS-1988 .
          Hide
          lemoene lemoene added a comment -

          I am splitting this issue into 2. Title is misleading by now too. Both small

          1. Error message resolution
          1. Line name as keyword
          Show
          lemoene lemoene added a comment - I am splitting this issue into 2. Title is misleading by now too. Both small Error message resolution Line name as keyword

            People

            • Assignee:
              Espurna Pau Soliva (Inactive)
              Reporter:
              lemoene lemoene
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Agile