Skip to content

converting input lists to np arrays#2504

Merged
bsipocz merged 1 commit into
astropy:mainfrom
jaymedina:mast-convert-lists
Aug 30, 2022
Merged

converting input lists to np arrays#2504
bsipocz merged 1 commit into
astropy:mainfrom
jaymedina:mast-convert-lists

Conversation

@jaymedina

Copy link
Copy Markdown
Contributor

astroquery.mast.Observations.get_product_list was mishandling lists because the assumed input was a numPy array. This led to incorrect requests being made to our API. This PR adds a conditional to catch for incoming lists to convert them into numPy arrays before handling.

Before:

image

After:

image

@jaymedina jaymedina requested review from bsipocz and ceb8 August 26, 2022 19:04
@codecov

codecov Bot commented Aug 26, 2022

Copy link
Copy Markdown

Codecov Report

Merging #2504 (dcc62fd) into main (0883e92) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #2504   +/-   ##
=======================================
  Coverage   62.99%   62.99%           
=======================================
  Files         133      133           
  Lines       17291    17293    +2     
=======================================
+ Hits        10892    10894    +2     
  Misses       6399     6399           
Impacted Files Coverage Δ
astroquery/mast/observations.py 76.62% <100.00%> (+0.20%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@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.

Please add a test and a changelog

@bsipocz bsipocz added this to the v0.4.7 milestone Aug 26, 2022
Comment thread astroquery/mast/observations.py
@pep8speaks

pep8speaks commented Aug 26, 2022

Copy link
Copy Markdown

Hello @jaymedina! 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 2022-08-29 15:14:42 UTC

@jaymedina jaymedina marked this pull request as draft August 26, 2022 23:12
@jaymedina jaymedina marked this pull request as ready for review August 29, 2022 15:20

@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.

All looks good, thanks!

@bsipocz bsipocz merged commit b95f1d0 into astropy:main Aug 30, 2022
jaymedina added a commit to jaymedina/jwebbinar_prep that referenced this pull request Sep 9, 2022
Updating `astroquery` dependency on `requirements.txt`. 

During our testing of the notebooks we found and fixed an `astroquery` bug (see astropy/astroquery#2504) which was merged, but will not be reflected in the latest `astroquery` release 0.4.6. This change will point users to the latest development version of `astroquery` which contains the recent bug fix.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants