Added get_query_payload kwarg to Ogle

[nkphysics]

Hey yall!

I added in the get_query_payload kwarg into the Ogle module per #2040.
Its a relatively small addition.
For the tests my thought process was to cover all the existing test cases payloads.
I wasn't sure about the nested lists because of the potential deprecation, but I figured there's no harm in including tests for it since its working right now.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.66%. Comparing base (3881ee9) to head (12d9357).
⚠️ Report is 12 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3533      +/-   ##
==========================================
- Coverage   72.67%   72.66%   -0.01%     
==========================================
  Files         219      219              
  Lines       20485    20478       -7     
==========================================
- Hits        14887    14880       -7     
  Misses       5598     5598              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[keflavich]

Looks good to me.

@nkphysics did anything specific prompt this? i.e., was there a particular bug you were hunting down that led you to this fix? I don't mind general PRs to improve the consistency of astroquery modules, but I suspect there was more purpose behind this one.

[nkphysics]

Oddly enough there wasn't any particular bug that prompted this.
I got a few deprecation warnings (this seems to be common but I only recently started dipping into ogle data) when using it which prompted me to start poking around and looking at the source code after get_query_payload=True didn't work.

I remembered that last year I came across #2040 when something happened with skyview. I don't quite remember fully what happened with that, but in #3318 I ended up adding get_query_payload to skyview, and just figured since I came across this again I'd do the same for ogle.

I went ahead an did a quick audit of which modules have/don't have get_query_payload kwargs and dropped the list in #2040.

Since I came across wanting/needing get_query_payload twice now, I figure that when I find some spare time I can dedicate to this, I may end up trying to add this into other modules where possible.
No promises, but seems worthwhile.

[bsipocz]

Thanks!

Overall looks good, but it would be better if the new kwarg was placed into the method where all the other kwargs are handled, so we can have it listed in the docstring.

[bsipocz]

I appreciate covering this with tests, but overall I think we should instead remove this mode of passing non coordinates altogether; as we added that deprecation warning 6 years ago :)

[bsipocz]

Since this method practically pulls in everything from _args_to_payload, I would say the new parameter needs to be added there; along with listing it in the docstring.

Alternatively, if you are up for a bit more cleanup, that method can totally be moved into this one. It is only called once as I rather dislike the obstructing nature of having *args and **kwargs in the signature along with some docstring hackery.

[bsipocz]

Thank you! I have one small suggestion and I see one remote test failure which I'll just fix and push in a commit, too so we don't hold up merging.

[nkphysics]

Sorry I completely overlooked the docs!

Thanks a bunch @bsipocz!

[bsipocz]

No worries. I need to get those remote tests run for PRs, right now it's a bit of a manual process.