BUG: JPLSpec failures with dev versions of dependencies

[bsipocz]

jplspec failures started to show with the dev versions of numpy/astropy. I haven't yet pinpointed down the reason, maybe @keflavich you have some better ideas?

(this one copied below comes up in CI, but if you run the remote tests for the module a lot more tests fail with similar problems)

=================================== FAILURES ===================================
_______________________________ test_input_multi _______________________________

    def test_input_multi():
    
        response = JPLSpec.query_lines_async(min_frequency=500 * u.GHz,
                                             max_frequency=1000 * u.GHz,
                                             min_strength=-500,
                                             molecule=r"^H[2D]O(-\d\d|)$",
                                             parse_name_locally=True,
                                             get_query_payload=True)
        response = dict(response)
>       assert set(response['Mol']) == set((18003, 19002, 19003, 20003, 21001))
E       assert set() == {18003, 19002... 20003, 21001}
E         Extra items in the right set:
E         20003
E         18003
E         21001
E         19002
E         19003
E         Full diff:
E         - {20003, 21001, 18003, 19002, 19003}
E         + set()

../../.tox/py312-test-alldeps-devdeps-cov/lib/python3.12/site-packages/astroquery/jplspec/tests/test_jplspec.py:68: AssertionError

[keflavich]

I don't have an immediate explanation. Can you point to the specific version combinations that trigger this?

[bsipocz]

I don't have an immediate explanation. Can you point to the specific version combinations that trigger this?

dev numpy+dev astropy, e.g. : tox -e py312-test-devdeps. If you try it on a mac, you need to mess a bit around with the mpl wheel as it cannot be picked up properly from the channel. (rename the osx wheel to 12_0, and then manually pip install it).

[bsipocz]

not sure what we should do with this one, it's quite annoyingly turning the CI for all PRs red for one of the jobs, and is indeed a real failure that only pops up with new versions.

[bsipocz]

OK, so to narrow it down, this is due to astropy dev, and shows with older python and numpy released versions, too.

cc @pllim

[bsipocz]

I think I managed to narrow it down. Still not sure why it popped up, but clearly the molecule lookup with that somewhat complicated regex is the culprit, but unfortunately #2901 is not a solution for it.

[pllim]

Hmm something to do with response['Mol'] so probably related to table column look-up behavior. You might have to do a git bisect on astropy. Nightly wheel upload was broken for a while and lots of commits went in lately, so nothing specific popped up in my mind.

[pllim]

Oh wait, you did a response = dict(response). So that casted a table to dictionary? Why?

[bsipocz]

Spot the nonsense here (and yes, some of the Table slicing has change, but it's not Table's fault).

def build_lookup():

    result = JPLSpec.get_species_table()
    keys = list(result[1][:])  # convert NAME column to list
    values = list(result[0][:])  # convert TAG column to list
    dictionary = dict(zip(keys, values))  # make k,v dictionary
    lookuptable = lookup_table.Lookuptable(dictionary)  # apply the class above

    return lookuptable

(And I don't know the answer to your question, I suppose doing a regex lookup on a dictionary rather than on a Table seemed like a good idea, though we shouldn't have let it go ahead with selection a column this particular way...)

[pllim]

but it's not Table's fault

Okay, phew. But if somehow astropy broke some backward compatibility in a non-sensible way, please do report upstream. Thanks for checking!

[bsipocz]

Okay, phew. But if somehow astropy broke some backward compatibility in a non-sensible way, please do report upstream.

I think this way of selecting a column makes no sense, so no worries about it upstream, but if you think it's a legitimate usecase then it has to be reported and looked at.

result[1][:]

[pllim]

I am really not sure if Tom Aldcroft had any conscious design for calls like result[1][:]. I am guessing the author wanted a view of the second row, but then why cast it to list? Surely there is a better way?

[bsipocz]

row

column, they wanted the second column based on the code comment as well as previous behaviour. weird way to go for it, when result['NAME'] would do just that.