Fix failing tests moved from TraP

[suvayu]

See commit 1292e6de382002744013b3b34af71c34e667c043

[HannoSpreeuw]

Executing python -m pytest test_accessors/test_casatable.py from tkp/tests in my local (outdated) TraP clone passes.

So no clue yet.

[HannoSpreeuw]

And I can reproduce the error - i.e. the not raising of an OSError - from an up-to-date PySE using hatch run test:pytest -vv test/test_casatable.py.

[suvayu]

Maybe something in PySE got updated after splitting up that supports this. I was going through the implementation, and my opinion is these two tests are just wrong. They assume a feature doesn't exist (read as casa table) but that doesn't seem to be the case.

[HannoSpreeuw]

Started separate debugging sessions in my local (outdated Trap) and current PySE. In tkp/accessors/__init__.py we have

Accessor = tkp.accessors.detection.detect(path)

And for test/test_casatable.py::TestLofarCasaImage that returns None.

For current PySE, however:

Accessor = {type} <class 'sourcefinder.accessors.aartfaaccasaimage.AartfaacCasaImage'>

This means that there is something different within accessors.detection or in its dependent modules.

[HannoSpreeuw]

So with my breakpoint in line 109 of detection.py , from

telescope = table.getkeyword('coords')['telescope']

I get

telescope = "UNKNOWN"

while the equivalent up-to-date PySE code gives

telescope = 'AARTFAAC'

[suvayu]

Right, so support was added. That is why I think the tests are wrong now, because it was meant for an earlier version when AARTFAAC wasn't supported.

[HannoSpreeuw]

Yes, that could be.

The comment

    # CasaImages can't be directly instantiated, since they don't provide the
    # DataAccessor interface.

also explains the intention of the test. Any CasaImage should therefore not be accessible using accessors.open.

The only thing I do not yet understand is that with the same Casacore 3.5.1 version there is still a difference in test failing between (outdated) Trap and current PySE.

[HannoSpreeuw]

There seems to be a difference in the file accessed for the tkp/tests/test_accessors/test_casatable.py unit test: tkp/tests/data/accessors/casa.table for TraP vs. test/data/aartfaac.table for the test/test_casatable.py unit test in PySE.

[HannoSpreeuw]

Now I see it, tkp/tests/test_accessors/test_casatable.py has casatable = os.path.join(DATAPATH, 'accessors/casa.table') while pytest test/test_casatable.py has casatable = os.path.join(DATAPATH, 'aartfaac.table').

Perhaps some typo in commit 39e8550?

[HannoSpreeuw]

I guess casa.table has not been copied into PySE. In Trap it is in /tkp/tests/data/accessors/. This directory has aartfaac.fits aartfaac.table ami-la.image casa.table kat7.image lofar.fits lofar.h5 missing_metadata.fits. All these files are in PySE's test/data/, except casa.table. We could also create a test/data/accessors subdirectory mimicking /tkp/tests/data/accessors/.

[suvayu]

I know what happened now. This is a bit confusing because PySE already has a casatable directory. I wasn't sure if that's the same so erred on the side of caution. And replaced with what seemed right to me (but I guess I didn't understand the test correctly). Anyway, if you take a look at the casa.table directory in TraP and suggest a more appropriate name, I can add it. -- Suvayu

Open source is the future. It sets us free.

On Fri, 25 Oct, 2024, 11:14 Hanno Spreeuw, @.***> wrote:

I guess casa.table has not been copied into PySE. In Trap it is in /tkp/tests/data/accessors/. This directory has aartfaac.fits aartfaac.table ami-la.image casa.table kat7.image lofar.fits lofar.h5 missing_metadata.fits. All these files are in PySE's test/data/, except casa.table. We could also create a test/data/accessors subdirectory mimicking /tkp/tests/data/accessors/.

— Reply to this email directly, view it on GitHub https://github.com/transientskp/pyse/issues/81#issuecomment-2437291787, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABYBJFKLEPUQKDML6SI2ELZ5IDWFAVCNFSM6AAAAABQNWK4M2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMZXGI4TCNZYG4 . You are receiving this because you authored the thread.Message ID: @.***>

[HannoSpreeuw]

Thanks, pls. let me take care of that after the weekend, I have some commits that are almost ready to push.