Skip to content

Check that parameters are valid when running criteria queries#3084

Merged
bsipocz merged 2 commits into
astropy:mainfrom
snbianco:ASB-28248-invalid-args
Aug 12, 2024
Merged

Check that parameters are valid when running criteria queries#3084
bsipocz merged 2 commits into
astropy:mainfrom
snbianco:ASB-28248-invalid-args

Conversation

@snbianco

Copy link
Copy Markdown
Contributor

Part of our efforts at MAST to prevent very large requests from Astroquery!

If a user attempts to use a criteria argument that doesn't exist in Observations.query_criteria or Catalogs.query_criteria, they will encounter an InvalidQueryError. If the provided keyword is similar to an expected keyword, the user will be prompted with the expected keyword in the error message.

@snbianco snbianco requested a review from bsipocz August 12, 2024 18:36
@snbianco snbianco marked this pull request as ready for review August 12, 2024 18:36
@bsipocz bsipocz added the mast label Aug 12, 2024

@bsipocz bsipocz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a related test failure, please check it locally:


self = <astroquery.mast.tests.test_mast_remote.TestMast object at 0x1277d7f80>

    def test_catalogs_query_criteria_invalid_keyword(self):
        # attempt to make a criteria query with invalid keyword
        with pytest.raises(InvalidQueryError) as err_no_alt:
>           Catalogs.query_criteria(catalog='tic', not_a_keyword='TESS')

astroquery/mast/tests/test_mast_remote.py:898: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
astroquery/utils/class_or_instance.py:25: in f
    return self.fn(obj, *args, **kwds)
astroquery/utils/process_asyncs.py:26: in newmethod
    response = getattr(self, async_method_name)(*args, **kwargs)
astroquery/utils/class_or_instance.py:25: in f
    return self.fn(obj, *args, **kwds)
astroquery/mast/collections.py:298: in query_criteria_async
    filters = self._current_connection.build_filter_set("Mast.Catalogs.Tess.Cone",
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <astroquery.mast.discovery_portal.PortalAPI object at 0x126b5eb70>
column_config_name = 'Mast.Catalogs.Tess.Cone', service_name = 'Mast.Catalogs.Filtered.Tic.Rows'
filters = {'not_a_keyword': 'TESS'}
caom_col_config = {'ALLWISE': {'autoFacetRule': 'never', 'excludeFromCharts': True, 'text': 'ALLWISE ID'}, 'APASS': {'autoFacetRule': 'n...00171661377, 'type': 'numeric'}, 'GAIA': {'autoFacetRule': 'never', 'excludeFromCharts': True, 'text': 'GAIA ID'}, ...}
mashup_filters = [], colname = 'not_a_keyword', value = ['TESS'], col_info = None

    def build_filter_set(self, column_config_name, service_name=None, **filters):
        """
        Takes user input dictionary of filters and returns a filterlist that the Mashup can understand.
    
        Parameters
        ----------
        column_config_name : string
            The service for which the columns config will be fetched.
        service_name : string, optional
            The service that will use the columns config, default is to be the same as column_config_name.
        **filters :
            Filters to apply. At least one filter must be supplied.
            Valid criteria are coordinates, objectname, radius (as in `query_region` and `query_object`),
            and all observation fields listed `here <https://mast.stsci.edu/api/v0/_c_a_o_mfields.html>`__.
            The Column Name is the keyword, with the argument being one or more acceptable values for that parameter,
            except for fields with a float datatype where the argument should be in the form [minVal, maxVal].
            For example: filters=["FUV","NUV"],proposal_pi="Osten",t_max=[52264.4586,54452.8914]
    
        Returns
        -------
        response : list(dict)
            The mashup json filter object.
        """
    
        if not service_name:
            service_name = column_config_name
    
        if not self._column_configs.get(service_name):
            self._get_col_config(service_name, fetch_name=column_config_name)
    
        caom_col_config = self._column_configs[service_name]
    
        mashup_filters = []
        for colname, value in filters.items():
    
            # make sure value is a list-like thing
            if np.isscalar(value,):
                value = [value]
    
            # Get the column type and separator
            col_info = caom_col_config.get(colname)
            if not col_info:
>               closest_match = difflib.get_close_matches(colname, caom_col_config.keys(), n=1)[0]
E               IndexError: list index out of range

astroquery/mast/discovery_portal.py:410: IndexError

closest_match = difflib.get_close_matches(colname, caom_col_config.keys(), n=1)[0]
error_msg = f"Filter '{colname}' does not exist. Did you mean '{closest_match}'?" if closest_match \
else f"Filter '{colname}' does not exist."
raise InvalidQueryError(error_msg)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given this changes the warning to an error, please add a short changelog for the users.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, tests should pass now, sorry about that!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries, this is what review is for.

@bsipocz bsipocz added this to the v0.4.8 milestone Aug 12, 2024

@bsipocz bsipocz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@bsipocz bsipocz merged commit d347559 into astropy:main Aug 12, 2024
@snbianco snbianco deleted the ASB-28248-invalid-args branch November 18, 2024 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants