transientskp / pyse

Python Source Extractor
BSD 2-Clause "Simplified" License
11 stars 5 forks source link

iscasa() errors when a string is supplied as path #54

Closed tmillenaar closed 4 months ago

tmillenaar commented 4 months ago

When porting over the accessors from tkp to use the accessors from pyse, I encountered an error when attempting to open a casacore file. The file is being opened by the test tests/test_inject.py::TestLofarCasaInject::test_no_injection in tkp. It tries to open the following file: https://github.com/transientskp/trap-test-data/tree/master/casatable/L55596_000TO009_skymodellsc_wmax6000_noise_mult10_cell40_npix512_wplanes215.img.restored.corr The open call is made on this line: https://github.com/transientskp/tkp/blob/8a19cd23c7141c66c1ee8e42295957bbcf809531/tests/test_inject.py#L55

It then errors with the following error:

*** Boost.Python.ArgumentError: Python argument types in
    Table.__init__(table, list, list, int, int, int)
did not match C++ signature:
    __init__(_object*, casacore::String, casacore::String, casacore::String, bool, casacore::IPosition, casacore::String, casacore::String, int, int, casacore::Vector<casacore::String, std::allocator<casacore::String> >, casacore::Vector<casacore::String, std::allocator<casacore::String> >)
    __init__(_object*, casacore::String, casacore::Record, casacore::String, casacore::String, int, casacore::Record, casacore::Record)
    __init__(_object*, std::vector<casacore::TableProxy, std::allocator<casacore::TableProxy> >, casacore::Vector<casacore::String, std::allocator<casacore::String> >, int, int, int)
    __init__(_object*, casacore::Vector<casacore::String, std::allocator<casacore::String> >, casacore::Vector<casacore::String, std::allocator<casacore::String> >, casacore::Record, int)
    __init__(_object*, casacore::String, casacore::Record, int)
    __init__(_object*, casacore::String, std::vector<casacore::TableProxy, std::allocator<casacore::TableProxy> >)
    __init__(_object*, casacore::TableProxy)
    __init__(_object*)

The one noteworthy difference I found is that pyse uses .encode(): https://github.com/transientskp/pyse/blob/91f8c116a59da2769eb78e379bc90a627b70bef4/sourcefinder/accessors/detection.py#L65 Where tkp does not: https://github.com/transientskp/tkp/blob/8a19cd23c7141c66c1ee8e42295957bbcf809531/tkp/accessors/detection.py#L64

I think this was fixed by @HannoSpreeuw when moving from python2 to python3 in this commit: https://github.com/transientskp/tkp/commit/a8c7d02cfe5ae5786812f904c4cc1cfb2f1e4a02

But it seems the fix never made it to pyse.

Is this correct? If so, let's fix it on pyse as well :)

Cheers, Timo

suvayu commented 4 months ago

I've tested the fix locally, but since the data is large, I did not include it in the repo yet. I would like to think on it a bit. In CI, this test will be skipped.

tmillenaar commented 4 months ago

Thanks for the quick release :)

Indeed the test data is quite large. Maybe we should rething the test repo a bit, now in TraP I download that test repo in the CICD every time. I would like it to be a lot smaller, but that is for another time.