Add flexible parameters to criteria #1659

olyoberdorf · 2020-02-28T19:35:29Z

This enhances query_criteria to allow additional arguments to pass through to the raw query. This enables users to call the helper method (query_criteria) but still have access to any new or less common parameters in the webservice.

#1658

pep8speaks · 2020-02-28T19:35:33Z

Hello @olyoberdorf! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-03-03 00:09:31 UTC

bsipocz · 2020-02-28T21:41:36Z

astroquery/gemini/core.py

@@ -156,9 +156,10 @@ def query_object(self, objectname, radius=0.3*units.deg):
        return self.query_criteria(objectname=objectname, radius=radius)

    @class_or_instance
-    def query_criteria(self, coordinates=None, radius=0.3*units.deg, pi_name=None, program_id=None, utc_date=None,
+    def query_criteria(self, *args, coordinates=None, radius=0.3*units.deg, pi_name=None, program_id=None, utc_date=None,


This is an API break, users could have used the kwargs as positional args before, and those will fails with then new added *args. So I would suggest to add it to the end as a new keyword, e.g. rawqueryargs (well, something more sensible, than this)

I've renamed them with the rawquery name prefix and moved args to the back. I ended up putting orderby sandwiched between them to prevent people using it via position. I didn't want future search terms to result in orderby sitting in the middle of the positonal args list.

Prior, orderby was being picked up by the *args, so the explicit add and handling is really just making the signature match the docs.

Thanks. We could also switch to keyword only args, definitely in the next refactoring release, but given the gemini module is relatively new, if you want you can make that change any time you wish.

bsipocz · 2020-02-28T21:42:09Z

astroquery/gemini/core.py

@@ -252,6 +253,17 @@ def query_criteria(self, coordinates=None, radius=0.3*units.deg, pi_name=None, p
                'PROCESSED_FLAT',
                'PROCESSED_FRINGE',
                'PROCESSED_ARC'
+        orderby : str, optional


this is not in the signature

per above, made it explicit in the signature

bsipocz · 2020-02-28T21:43:05Z

astroquery/gemini/core.py

@@ -343,7 +371,9 @@ def query_raw(self, *args, **kwargs):
            The list of parameters to be passed via the query path to the webserver
        kwargs :
            The dictionary of parameters to be passed by name=value within the query
-            path to the webserver
+            path to the webserver.  The `orderby` key value pair has a special


use double backticks. sphinx will try to convert single ones to links and fails to do it for any parameters

bsipocz · 2020-02-28T23:10:19Z

astroquery/gemini/tests/test_remote.py

+    def test_observations_query_region(self):
+        """ test query against a region of the sky against actual archive """
+        result = gemini.Observations.query_region(coords, radius=0.3 * units.deg)
+        assert isinstance(result, Table)


some check on the content of the Table would be nice in the tests, just to make sure data is returned rather than an empty table (which is the default behaviour for a nonsensical query, if I see correctly)

I've added a length > 0 check. That is still pretty naive but at least it will prevent the default causing a false pass.

docs/gemini/gemini.rst

bsipocz · 2020-02-28T23:11:16Z

docs/gemini/gemini.rst

@@ -83,6 +83,20 @@ and the program ID.  For a complete list of available search fields, see
                          0.0           Full Frame                         -- Gemini-North    True ...
                          0.0           Full Frame                         -- Gemini-North    True ...

+In addition, the criteria query can accept additional parameters via the `args` and `kwargs` optional parameters.


double backticks

also, might be nice to have another extra example with passing in the kwargs

also, might be nice to have another extra example with passing in the kwargs

I missed this but made a note of it for next time

bsipocz · 2020-02-28T23:16:22Z

Some minor comments, otherwise this looks good.

@olyoberdorf - could you also rebase to get rid of the merge commit?

olyoberdorf · 2020-03-02T18:43:41Z

Some minor comments, otherwise this looks good.

@olyoberdorf - could you also rebase to get rid of the merge commit?

Thanks for looking so quickly - I will make those edits and rebase and post back.

bsipocz

I could spot these two more cases of single vs double backtick usage. Otherwise once CI is all green this is ready to go in.

bsipocz · 2020-03-02T22:44:57Z

astroquery/gemini/core.py

@@ -252,18 +255,45 @@ def query_criteria(self, coordinates=None, radius=0.3*units.deg, pi_name=None, p
                'PROCESSED_FLAT',
                'PROCESSED_FRINGE',
                'PROCESSED_ARC'
+        orderby : str, optional
+            Indicates how the results should be sorted.  Values should be like the ones used
+            in the archive website when sorting a column.  For example, `data_label_desc` would


Suggested change

in the archive website when sorting a column. For example, `data_label_desc` would

in the archive website when sorting a column. For example, ``data_label_desc`` would

bsipocz · 2020-03-02T22:45:28Z

docs/gemini/gemini.rst

+In addition, the criteria query can accept additional parameters via the ``*rawqueryargs`` and ``**rawquerykwargs`` 
+optional parameters.
+
+The `orderby` parameter can be used to pass the desired sort order.


Suggested change

The `orderby` parameter can be used to pass the desired sort order.

The ``orderby`` parameter can be used to pass the desired sort order.

…ria to be passed down to the URL Added support for ordering results via orderby parameter (properly used as URL query argument) fixed documentation to latest tweaks for added criteria query args and sort order Changelog entry for Gemini enhancements alphabetizing my changelog entry by module name tagging tests as remote

bsipocz · 2020-03-04T07:02:43Z

Thanks @olyoberdorf!

astropy-bot bot added the gemini label Feb 28, 2020

bsipocz added this to the v0.4.1 milestone Feb 28, 2020

bsipocz reviewed Feb 28, 2020

View reviewed changes

olyoberdorf force-pushed the add_flexible_parameters_to_criteria branch 2 times, most recently from 409fd93 to d7b8e43 Compare March 2, 2020 21:49

bsipocz reviewed Mar 2, 2020

View reviewed changes

bsipocz added the merge-when-ci-passes label Mar 2, 2020

olyoberdorf force-pushed the add_flexible_parameters_to_criteria branch from d7b8e43 to 491b2a3 Compare March 3, 2020 00:09

bsipocz approved these changes Mar 4, 2020

View reviewed changes

bsipocz merged commit f452576 into astropy:master Mar 4, 2020

olyoberdorf deleted the add_flexible_parameters_to_criteria branch May 4, 2020 22:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add flexible parameters to criteria #1659

Add flexible parameters to criteria #1659

olyoberdorf commented Feb 28, 2020

pep8speaks commented Feb 28, 2020 •

edited

Loading

bsipocz Feb 28, 2020

olyoberdorf Mar 2, 2020

bsipocz Mar 2, 2020

bsipocz Feb 28, 2020

olyoberdorf Mar 2, 2020

bsipocz Feb 28, 2020

olyoberdorf Mar 2, 2020

bsipocz Feb 28, 2020

olyoberdorf Mar 2, 2020

bsipocz Feb 28, 2020

bsipocz Feb 28, 2020

olyoberdorf Mar 2, 2020

olyoberdorf Mar 3, 2020

bsipocz commented Feb 28, 2020

olyoberdorf commented Mar 2, 2020 •

edited

Loading

bsipocz left a comment

bsipocz Mar 2, 2020

olyoberdorf Mar 4, 2020

bsipocz Mar 2, 2020

olyoberdorf Mar 4, 2020

bsipocz commented Mar 4, 2020

	in the archive website when sorting a column. For example, `data_label_desc` would
	in the archive website when sorting a column. For example, ``data_label_desc`` would

	The `orderby` parameter can be used to pass the desired sort order.
	The ``orderby`` parameter can be used to pass the desired sort order.

Add flexible parameters to criteria #1659

Add flexible parameters to criteria #1659

Conversation

olyoberdorf commented Feb 28, 2020

pep8speaks commented Feb 28, 2020 • edited Loading

Comment last updated at 2020-03-03 00:09:31 UTC

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bsipocz commented Feb 28, 2020

olyoberdorf commented Mar 2, 2020 • edited Loading

bsipocz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bsipocz commented Mar 4, 2020

pep8speaks commented Feb 28, 2020 •

edited

Loading

olyoberdorf commented Mar 2, 2020 •

edited

Loading