Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Perform unit conversion in svo_fps get_filter_index() #2444

Merged
merged 4 commits into from
Jun 17, 2022

Conversation

eerovaher
Copy link
Member

The SvoFpsClass.get_filter_index() method allows the user to specify wavelength limits. Previously the code assumed that they are always given in angstroms. Now the code converts them to angstroms, allowing the user to specify the wavelengths using any length unit and leading to an early unit conversion error if the limits do not have length units.

Fixes #2443

One of the remote tests requested data for all possible wavelength
values, unsurprisingly causing a timeout.
`SvoFpsClass.get_filter_index()` assumes that the wavelength limits are
in angstroms. This leads to unexpected results if different units are
used, as revealed by the modified test.
`SvoFpsClass.get_filter_index()` now accepts the wavelength limits in
any length unit, not just in angstroms.
@codecov
Copy link

codecov bot commented Jun 17, 2022

Codecov Report

Merging #2444 (23b8ad1) into main (2543cc2) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #2444   +/-   ##
=======================================
  Coverage   62.90%   62.90%           
=======================================
  Files         133      133           
  Lines       17291    17291           
=======================================
  Hits        10877    10877           
  Misses       6414     6414           
Impacted Files Coverage Δ
astroquery/svo_fps/core.py 93.93% <100.00%> (ø)

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@bsipocz bsipocz added this to the v0.4.7 milestone Jun 17, 2022
@bsipocz bsipocz merged commit f5ff578 into astropy:main Jun 17, 2022
@bsipocz
Copy link
Member

bsipocz commented Jun 17, 2022

Thanks @eerovaher!

As you have dug a bit into this module already, would you have any time or energy to fix the other remote failure about the missing error? Then we could tick this module out from #2203.

@eerovaher eerovaher deleted the svo-unit-conversion branch June 17, 2022 12:27
@bsipocz bsipocz mentioned this pull request Jun 20, 2022
28 tasks
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.

SvoFps.get_filter_index unit conversion issue?
2 participants