Instrument import interface: Sysmex XT4000i #537
Conversation
| # We test in import functions if the file was uploaded | ||
| try: | ||
| if self._encoding: | ||
| import codecs |
There was a problem hiding this comment.
Please add this import on the top of the module
| # Some Instruments can generate files with different encodings, so we | ||
| # may need this parameter | ||
| self._separator = separator | ||
| self._encoding = encoding |
There was a problem hiding this comment.
Why does these variables have to be prefixed with an _? Just a style question, but we want that our code looks nice and not like a regular expression (if possible).
There was a problem hiding this comment.
Just to tell the dev that these variables are meant to be private, only for instance's internal use. Just want to make a bit difficult for the dev to do something like instance.encoding='wtf'
| f = open(infile.name, 'rU') | ||
| except AttributeError: | ||
| f = infile | ||
| for line in f.readlines(): |
There was a problem hiding this comment.
This file never gets closed. I'm not sure how this affects Memory in the long run, so I would prefer to use a context manager here, e.g. with file.open(...) and read the lines somewhere into a variable. Probably best to do that in a separate method. e.g. lines = self.read(infile.name). There you could also preprocess the lines, e.g. with lines.strip or similar
| except AttributeError: | ||
| f = infile | ||
| for line in f.readlines(): | ||
| self._numline += 1 |
There was a problem hiding this comment.
@ramonski As far as I checked self._numline is used to show the line number in the logs
| elif sline[0] == SEGMENT_RESULT: | ||
| return self._handle_result_line(sline) | ||
| elif sline[0] == SEGMENT_EOF: | ||
| return self._handle_eof(sline) |
There was a problem hiding this comment.
You are accessing here multiple times the first element of the header, therefore I would create a variable like segment = sline[0] and then you could think of a mapping of segment -> handler to get it immediately, e.g.
handlers = {
SEGMENT_HEADER: self._handle_header,
...
}
handler = hanlers.get(segment)
...| tbex = '' | ||
| try: | ||
| importer.process() | ||
| except: |
| allowed_ar_states=status, | ||
| allowed_analysis_states=None, | ||
| override=over, | ||
| instrument_uid=instrument) |
| elif sample == 'clientsid': | ||
| sam = ['getClientSampleID'] | ||
| elif sample == 'sample_clientsid': | ||
| sam = ['getSampleID', 'getClientSampleID'] |
There was a problem hiding this comment.
better use a dictionary mapping here
| elif override == 'override': | ||
| over = [True, False] | ||
| elif override == 'overrideempty': | ||
| over = [True, True] |
There was a problem hiding this comment.
better use a dictionary mapping here
| if artoapply == 'received': | ||
| status = ['sample_received'] | ||
| elif artoapply == 'received_tobeverified': | ||
| status = ['sample_received', 'attachment_due', 'to_be_verified'] |
There was a problem hiding this comment.
better use a dictionary mapping here
…de, status and sample variables values
Description of the issue/feature this PR addresses
IMPORTANT: #536 needs to be accepted first
Import interface for 'Sysmex XT-4000i' instrument, a fluorescence flow cytometry for high quality haematology analysis. It uses 'Sysmex XT-1800i's Parser (#536), since they are in the same format.
https://www.sysmex-mea.com/products/xt-4000i-919.html
--
I confirm I have tested this PR thoroughly and coded it according to PEP8
and Plone's Python styleguide standards.