Fixed failing PANSTARRS Catalog queries

[jaymedina]

Summary

Resolves #2709
Resolves #2716

Users were reporting issues where PANSTARRS queries were failing when columns were specified, or any column sorting was requested with a sorting direction.

The solution I found was:

• To pass the parameters as JSON when making requests to the PANSTARRS API (see changes in collections.py)
• Call upon _build_catalogs_params when use_json is true for PANSTARRS queries (see changes in services.py). This call updates the syntax of the parameters being passed so that users can use criteria decorators (e.g. sort_by=[("asc", "distance")]), but setting use_json to true would prevent these decorators from passing because the synax wasnt being updated (params was left as the original params dictionary so it can be converted to a JSON object in the request). This didn't seem to be a problem before, but now that the PANSTARRS API needs the parameters to be passed as JSON dictionaries, this call needed to be made for both use_json = False AND True. Notice how none of the MastMissions examples have criteria decorators. This is because use_json is True so the params are passed in a dictionary without updating the syntax. That's something we can improve upon MastMissions the way I've done here with PANSTARRS, but is out of scope of this PR, so I'll make a separate issue about it.

---

Task list

• Resolve 422 Unprocessable Entity error in PANSTARRS Catalog queries
• Add a unit test to catch potential errors from queries like this in the future
• CHANGELOG entry

---

Screenshots

When sorting direction is specified, (direction, criteria) tuples can be passed as a single tuple or list of tuples:

The changes in this PR do not break non-PANSTARRS queries to the Catalogs API:

The changes in this PR do not break non-PANSTARRS queries that request specific columns and/or sorting:

Below are screenshots (taken May 4, 2023) showing the results of the queries that reported errors, after fix:

#2709:

#2716:

[keflavich]

@jaymedina your comment says there are screenshots, but I'm not seeing them; were they unintentionally omitted?

[jaymedina]

@jaymedina your comment says there are screenshots, but I'm not seeing them; were they unintentionally omitted?

No I just hadnt added them yet, I've updated the description now.

[jaymedina]

This is ready for a pass through. There are 2 CI tests failing due to something upstream. I triggered the CI tests in my other open PR (#2693) and it failed with the same errors, so I'm going to make a separate PR to resolve that issue and rebase this branch (and my other branch) onto main once the fix has been merged. This code is ready to go, though. Let me know if you have any questions/comments! Thanks

[jaymedina]

This call updates the syntax of the parameters so it's readable in the backend. It needs to be made here so that users can add criteria decorators to their PANSTARRS queries, despite it being an API that requires params to be passed as JSON. It returns the params in list format, so I then have to convert it back to dictionary so that it can be dumped as a JSON dict later in the request.

[jaymedina]

This else statement will ignore all the logic from above if the service is not the PANSTARRS API. This is because the MastMissions API also requires params to be passed as JSON, but doesn't jive with the parameter syntax transformation that happens above. Namely because of the way the coordinates are passed for MastMissions.

[jaymedina]

For the unit test, I'm using one of the queries that was reported to fail by one of our users. It includes the columns and sort_by arguments, both arguments were causing the 422 error observed by both users. This unit test will catch any future issues with the criteria syntax, should they arise.

[keflavich]

I think we'll wait to merge this til the dev dependency issue is resolved? Unless that one looks like it's going to take a long time.

[jaymedina]

I think we'll wait to merge this til the dev dependency issue is resolved? Unless that one looks like it's going to take a long time.

I also think it's best to wait until I open up a PR addressing those tests and get that merged in. I'll get started on that later today. Thanks for the review!

[keflavich]

@jaymedina #2730 and #2729 fixed CI - rebase?

[eerovaher]

This pull request claims to address both #2709 and #2716, but because it didn't properly follow GitHub's syntax for linking multiple issues to a pull request then only #2709 was closed when it was merged and #2716 remains open.

[bsipocz]

Thanks for the heads up.