search

ProteoWizard / pwiz

The ProteoWizard Library is a set of software libraries and tools for rapid development of mass spectrometry and proteomic data analysis software.
http://proteowizard.sourceforge.net/
Apache License 2.0
210 stars 97 forks source link

Fix TestExportIsolationList #3014

Closed nickshulman closed 1 month ago

brendanx67 commented 1 month ago

Can you provide a comment explaining how this got broken and why the more permissive parsing is now desirable?

nickshulman commented 1 month ago

Can you provide a comment explaining how this got broken and why the more permissive parsing is now desirable?

The NumberStyles argument is not important, but I needed to pass in something in order to be able to call the "TryParse" overload that takes a CultureInfo.

The only actual change is the passing in "_cultureInfo" (which is set to "CultureInfo.InvariantCulture").

That is, the implementation of the two-argument float.TryParse looks like this:

    public static bool TryParse(string s, out float result)
    {
      return float.TryParse(s, NumberStyles.Float | NumberStyles.AllowThousands, NumberFormatInfo.CurrentInfo, out result);
    }
brendanx67 commented 1 month ago

So, can I assume this came up because we discovered the RT windows for Agilent were actually RT deltas, making it necessary to 1/2 the window values? And that caused round numbers to gain a decimal point? e.g. 5/2 = 2.5. I guess also there is the possibility of 0.1 for "RT (min)".

Keep explaining. That the field is name "RT Window (min)" makes me a bit nervous, since the code comment I saw called it "RT Delta (min)" and we are now halving it, which seems appropriate for a delta but not a window.

nickshulman commented 1 month ago

Keep explaining.

Can you take this over? I'm not near a computer anymore. I was just trying to fix this test breaking in French. If this test has bigger problems I can't fix it for a while.