search

frallain / pymsfilereader

Thermo MSFileReader Python bindings
MIT License
67 stars 25 forks source link

Reduce Code Duplication #2

Closed mobiusklein closed 5 years ago

mobiusklein commented 7 years ago

This PR contains a reduction in code duplication for creating the COM object, for delayed loading of the required DLL, single definitions of the namedtuple types used rather than re-creating them every time they're used, and more descriptive error messages when failure to load occurs due to platform incompatibility.

It also modifies the test suite to gracefully fall back to Python 2 exception names when the more specific Python 3 exception names are not available, and changed the the GetVersion test to instead simply test that the version string has the right textual form, rather than explicitly hard-coding the version string expected, which is impractical to assume will always be the same when the DLL being loaded is not specified by the test environment.

Also, as mentioned elsewhere, I committed the cardinal sin of white space reformatting.

Passes all tests in the test suite.

frallain commented 5 years ago

@mobiusklein Nice, I'll have finally a look, thanks for this.

frallain commented 5 years ago

@mobiusklein Thanks for this, it gave me some inspirations 2 years ago but I eventually decided to take your way since you have higer standards. Could you please tell me what was the OS, python version and architecture, msfilereader version and comtypes version you used? test_GetFilters gave me a different result, but even without your code, I am trying to find the root cause. Thanks in advance

mobiusklein commented 5 years ago

It was a long time ago, so the specifics are a bit hazy, but I was running tests with Python 2.7, either Windows 8 or 10, and MSFileReader 3.1 I think.

I eventually vendored my fork into another of my projects https://github.com/mobiusklein/ms_deisotope where I use it today and haven't seen any issues, although I don't use GetFilters.

I also wrote a wrapper for the new pure .NET reader along the same lines, which I noticed occasionally gave differing levels of detail in the filter strings it returned. Is this problem you noticed with GetFilters with the newest version of MSFileReader?

frallain commented 5 years ago

I just found out that GetFilters gives slightly different results in versions 3.0 compared to 3.1SP2, I will document this. Also, I've not been able yet to make 3.1SP4 work. It seems like it is not registered in the COM server, and registering myself with %systemroot%\SysWoW64\regsvr32 "C:\Program Files\Thermo\MSFileReader\XRawfile2_x64.dll" with admin permissions gives me

The module "C:\Program Files\Thermo\MSFileReader\XRawfile2_x64.dll" failed to load.
Make sure the binary is stored at the specified path or debug it to check for problems with the binary or dependent .DLL files.
The specified module could not be found.

Could you manage to make it work?

mobiusklein commented 5 years ago

Sorry, I haven't had a chance to look at this recently. I might get to it this weekend. Do you know what, if anything, was supposed to have changed between 3.1SP2 and 3.1SP4?

frallain commented 5 years ago

No idea about the changes between SP2 and SP4. I get the same unit tests results. I'll try on a more real world file.

--

François ALLAIN +353 83 470 6341 +33 651399906