Develop catalogues #6
Conversation
astroquery/eso/__init__.py
Outdated
| @@ -19,7 +19,9 @@ class Conf(_config.ConfigNamespace): | |||
| tap_url = _config.ConfigItem( | |||
There was a problem hiding this comment.
Rename tap_url --> tap_obs_url and its explanation as well, so that it is symmetrical with the config entry for catalogues.
astroquery/eso/core.py
Outdated
| row_limit_plus_one = self.ROW_LIMIT + 1 | ||
|
|
||
| table_with_an_extra_row = tap.search(query=query_str, maxrec=row_limit_plus_one).to_table() | ||
| table_with_an_extra_row = tap.search(query=query_str, |
There was a problem hiding this comment.
Note to myself: Either this 4-lines change 1) has no effect; 2) is wrong; or 3) the previous code is wrong. Double check...
There was a problem hiding this comment.
@ashleythomasbarnes, you can safely discard these changes (i.e. keep the original code). Explanation:
The proposed code change has no effect. It seems that the AI didn't understand the purpose of the if clause, and it was natural for it to turn it into a if...else clause.
The original if clause is only a safeguard against increasing an infinite row limit. It was introduced to fix a bug appearing when the user set ROW_LIMIT = 0. If we want to sound very mathematically elegant, we can say that it simply derives from the fact that ∞ +1 = ∞
astroquery/eso/core.py
Outdated
| ) | ||
|
|
||
| log.debug(f"Querying from {self._tap_url()}") | ||
| tap_url = self._tap_url() if which_tap == "tap_obs" else conf.tap_cat_url |
There was a problem hiding this comment.
here... This is another instance of the situation not being symmetrical between tap_cat and tap_obs: One is read from self._tap_url(); the other from the config directly. It is OK to have tap_obs set as a default, but for the rest tap_cat and tap_obs should be symmetrical. Do we have a good argument for them NOT being treated the same?
astroquery/eso/core.py
Outdated
| f"\nNumber of records present in the table {table_name}:\n{num_records}\n") | ||
|
|
||
| @unlimited_maxrec | ||
| @deprecated_renamed_argument('cache', None, since='0.4.12') |
There was a problem hiding this comment.
No need for deprecating anything here, since it is a new function. We can remove the cache argument.
| _ = cache # We're aware about disregarding the argument | ||
| schema = _EsoNames.catalogue_schema | ||
|
|
||
| query_str = (f"SELECT table_name FROM TAP_SCHEMA.tables as ref " |
There was a problem hiding this comment.
Note to myself - double check the queries of this function with Alberto
astroquery/eso/utils.py
Outdated
| from dataclasses import dataclass | ||
| from typing import Dict, List, Optional, Union | ||
|
|
||
| import numpy as np |
There was a problem hiding this comment.
Numpy is not used. This line should be removed.
There was a problem hiding this comment.
Four tests are failing:
test_list_catalogues_returns_tabletest_query_catalogues_unknown_collection_raisestest_query_catalogues_help_returns_tabletest_query_catalogues_cone_uses_ucd_columns
(https://github.com/eso/astroquery/actions/runs/21284643923/job/61262678496)
2 participants
New PR from #4 to help @juanmcloaiza for review...
Stripped back now, only query_catalogue and list_catalogues that mirror functionality of other functions included.