Open JJSong0805 opened 3 years ago
Hi @JJSong0805, thanks for your interest.
I don't yet understand why your proposed changes are necessary—I think FlowCal should successfully read a FCS3.0-or-greater file if $BEGINSTEXT
, $ENDSTEXT
, $BEGINANALYSIS
, and $ENDANALYSIS
are specified as described in the standard, as I read it. Do you have a FCS 3.0-or-greater file that fails to load because of one of those keyword-value pairs? If so, can you upload a few files for me to look at?
Also, the except
statements in your proposed code would need to be more precise (what Exception is being caught?). And the pull request would need to branch from and merge into the develop
branch, not the master
branch.
Hi @JS3xton, sorry for the late reply.
The attached files are the pseudo files I created to help illustrate the situation we ran into. Lack_BEGINSTEXT.fcs
and Lack_BEGINANALYSIS
are FCS 3.0 files but lack BEGINSTEXT/ENDSTEXT and BEGINANALYSIS/ENDANALYSIS respectively. We understand by the standards of FCS 3.0 these attributes are all required so it totally makes sense to check if they exist. But in the real settings of different labs, we really can't say each machine are set to generate the FCS files following all the standards. We just get a batch of FCS 3.0 data from a famous hospital group, but many of these files lack the attributes I mentioned above, which hinder us from using the current FlowCal to read these files. Therefore, I propose changing the above attribute check from "hard check" to "soft check" and set these missing attributes to 0, which will not affect the result at all. This way, users can see the underlying issues of the data (by the warning messages) but still can read them with FlowCal.
Also, I specified the Error type after except
, and change the target branch to develop
.
$BEGINSTEXT and $ENDSTEXT are allowed to be empty in the TEXT segment, and the empty value will be treated as 0. Warning is generated to report the issue.
$BEGINANALYSIS and $ENDANALYSIS are allowed to be empty in the TEXT segment, and the empty value will be treated as 0. Warning is generated to report the issue.
These changes are made because we found most of the labs, no matter they are using BD's or Beckmen Coulter's intruments, does not put meaningful values in these keywords in the TEXT segment. Most of them are zeros and the rest of them don't have these keys in the TEXT segment, which actually shouldn't affect reading either TEXT or ANALYSIS segment in the fcs 3.0 (or above) files because these information are always in the Header segment. Therefore, we think it'd better raise warning instead of raising error to make it work even if these keywords are missing in the TEXT segment.