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

Remove some useless ACQ star warnings #437

Merged
merged 2 commits into from
Feb 9, 2024
Merged

Conversation

jeanconn
Copy link
Contributor

@jeanconn jeanconn commented Nov 28, 2023

Description

Remove some warnings on acquisition stars. These per-star warnings are no longer relevant as they are issues that are included in the proseco calculation of acquisition probability.

Remove

  1. "Magnitude. Acq star" warnings for acq stars fainter than red or yellow (calculated) mag limits
  2. "Acq Off (padded) CCD" warning

These warnings will be removed for ACQ-only stars but continue for BOT

  1. "Magnitude. MAXMAG - MAG < 0.3"
  2. "Magnitude. MAXMAG %.2f not within 0.1 mag of %.2f" (for maxmag not close to expected value)

Fixes #434

Interface impacts

Testing

Unit tests

  • Linux
(ska3-masters) jeanconn-fido> git rev-parse HEAD
ca4812dca4e2b459ed7291c8f03e846c1416a846
(ska3-masters) jeanconn-fido> pytest
======================================================================= test session starts ========================================================================
platform linux -- Python 3.10.8, pytest-7.2.1, pluggy-1.0.0
rootdir: /proj/sot/ska/jeanproj/git/starcheck
plugins: timeout-2.1.0, anyio-3.6.2
collected 1 item                                                                                                                                                   

starcheck/tests/test_utils.py .                                                                                                                              [100%]

========================================================================= warnings summary =========================================================================
../../../../../../fido.real/conda/envs/ska3-masters/lib/python3.10/site-packages/numexpr/expressions.py:21
../../../../../../fido.real/conda/envs/ska3-masters/lib/python3.10/site-packages/numexpr/expressions.py:21
  /fido.real/conda/envs/ska3-masters/lib/python3.10/site-packages/numexpr/expressions.py:21: DeprecationWarning: distutils Version classes are deprecated. Use packaging.version instead.
    _np_version_forbids_neg_powint = LooseVersion(numpy.__version__) >= LooseVersion('1.12.0b1')

../../../../../../fido.real/conda/envs/ska3-masters/lib/python3.10/site-packages/jupyter_client/connect.py:27
  /fido.real/conda/envs/ska3-masters/lib/python3.10/site-packages/jupyter_client/connect.py:27: DeprecationWarning: Jupyter is migrating its paths to use standard platformdirs
  given by the platformdirs library.  To remove this warning and
  see the appropriate new directories, set the environment variable
  `JUPYTER_PLATFORM_DIRS=1` and then run `jupyter --paths`.
  The use of platformdirs will be the default in `jupyter_core` v6
    from jupyter_core.paths import jupyter_data_dir

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
================================================================== 1 passed, 3 warnings in 32.56s

Independent check of unit tests by [REVIEWER NAME]

  • [PLATFORM]:

Functional tests

Several recent load weeks run. Output and diffs relative to master at https://icxc.cfa.harvard.edu/aspect/test_review_outputs/starcheck/starcheck-pr437/ . Diffs show expected changes.

@jeanconn jeanconn changed the title WIP: Remove some useless ACQ star warnings Remove some useless ACQ star warnings Jan 18, 2024
@jeanconn jeanconn marked this pull request as ready for review January 18, 2024 02:36
@jeanconn
Copy link
Contributor Author

jeanconn commented Feb 2, 2024

I suppose, technically, this PR should also update the checklist to just change ACA-041 to a guide-only check.

Copy link
Member

@taldcroft taldcroft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@jeanconn
Copy link
Contributor Author

jeanconn commented Feb 2, 2024

What are your thoughts on ACA-041 @taldcroft ?

@taldcroft
Copy link
Member

Sure, cut out the AS: bit.

@jeanconn jeanconn merged commit 9feb420 into master Feb 9, 2024
@jeanconn jeanconn deleted the remove-more-warnings branch February 9, 2024 14:04
This was referenced Mar 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove "MAXMAG 10.94 not within 0.1 mag of 11.20. (MAXMAG-MAG=0.89)" warning
2 participants