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

Fix roll optimize deepcopy issue and fix warning in tests #220

Merged
merged 3 commits into from
Jan 6, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
repos:
- repo: https://github.com/astral-sh/ruff-pre-commit
# Ruff version.
rev: v0.2.2
rev: v0.8.5
Copy link
Contributor

Choose a reason for hiding this comment

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

0.8.6 was released two hours ago :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Hah.

hooks:
# Run the linter.
- id: ruff
Expand Down
21 changes: 18 additions & 3 deletions sparkles/roll_optimize.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
"""

import warnings
from copy import deepcopy

import numpy as np
import ska_sun
Expand Down Expand Up @@ -367,8 +366,24 @@ def get_roll_options(

att_targ = self.att_targ

# Special case, first roll option is self but with obsid set to roll
acar = deepcopy(self)
# Special case, first roll option is self but with obsid set to roll. Note that
# the "copy" from the line below is not a full deep copy of the table meta
# because it runs the __init__ method. This initializes the MetaAttributes to
# their default states instead of copying them. This is *required* for this code
# to work.
#
# As a development note, this used to be deepcopy(self), but in astropy 7.0 the
# implementation changed so that the meta dict was explicitly deepcopied after
# the __init__ call. This caused the MetaAttributes to be set to the original
# self values and the code to fail. The line below is the astropy 6.0 code for
# implementing deepcopy().
#
# See also https://github.com/sot/sparkles/pull/220/files#r1903130172 for more
# discussion and some clarification (maybe). One idea is to make do
# deepcopy(self) and not re-run check_catalog(); OR, make a brand new catalog
# from self.call_args.
acar = self.copy(copy_data=True)
taldcroft marked this conversation as resolved.
Show resolved Hide resolved

check_catalog(acar)
acar.is_roll_option = True
roll_options = [
Expand Down
12 changes: 6 additions & 6 deletions sparkles/tests/test_find_er_catalog.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,9 @@ def test_get_candidate_and_filter_stars(proseco_agasc_1p7, ATT, T_CCD, DATE):
def test_find_er_catalog_minus_2_pitch_bins(proseco_agasc_1p7, ACA, ATTS):
# Try it all for the bad field near PKS 0023-26
acar, att_opts = find_er_catalog(ACA, ATTS, alg="pitch_bins")
# import pprint; pprint.pprint(att_opts[TEST_COLS].pformat_all(), width=100)
# import pprint; pprint.pprint(att_opts[TEST_COLS].pformat(), width=100)
assert acar is att_opts["acar"][8]
assert att_opts[TEST_COLS].pformat_all() == [
assert att_opts[TEST_COLS].pformat() == [
"dpitch dyaw count_9th count_10th count_all count_ok n_critical att ",
"------ ----- --------- ---------- --------- -------- ---------- -----------------",
" -0.01 3.10 4.18 6.00 5.65 True 2 7.66 -25.21 29.3",
Expand All @@ -112,9 +112,9 @@ def test_find_er_catalog_minus_2_pitch_bins(proseco_agasc_1p7, ACA, ATTS):

def test_find_er_catalog_minus_2_count_all(proseco_agasc_1p7, ACA, ATTS):
acar, att_opts = find_er_catalog(ACA, ATTS, alg="count_all")
# import pprint; pprint.pprint(att_opts[TEST_COLS].pformat_all(), width=100)
# import pprint; pprint.pprint(att_opts[TEST_COLS].pformat(), width=100)
assert acar is att_opts["acar"][8]
assert att_opts[TEST_COLS].pformat_all() == [
assert att_opts[TEST_COLS].pformat() == [
"dpitch dyaw count_9th count_10th count_all count_ok n_critical att ",
"------ ----- --------- ---------- --------- -------- ---------- -----------------",
" -0.01 3.10 4.18 6.00 5.65 True -- 7.66 -25.21 29.3",
Expand All @@ -134,9 +134,9 @@ def test_find_er_catalog_minus_2_count_all(proseco_agasc_1p7, ACA, ATTS):

def test_find_er_catalog_minus_2_input_order(proseco_agasc_1p7, ACA, ATTS):
acar, att_opts = find_er_catalog(ACA, ATTS, alg="input_order")
# import pprint; pprint.pprint(att_opts[TEST_COLS].pformat_all(), width=100)
# import pprint; pprint.pprint(att_opts[TEST_COLS].pformat(), width=100)
assert acar is att_opts["acar"][8]
assert att_opts[TEST_COLS].pformat_all() == [
assert att_opts[TEST_COLS].pformat() == [
"dpitch dyaw count_9th count_10th count_all count_ok n_critical att ",
"------ ----- --------- ---------- --------- -------- ---------- -----------------",
" -0.01 3.10 4.18 6.00 5.65 True 2 7.66 -25.21 29.3",
Expand Down
2 changes: 1 addition & 1 deletion sparkles/tests/test_yoshi.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,4 +171,4 @@ def test_acar_from_ocat(proseco_agasc_1p7):
]

cols = ("idx", "slot", "id", "type", "mag", "yang", "zang")
assert acar[cols].pformat_all() == exp
assert acar[cols].pformat() == exp
Loading