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

Change namespace package imports and update answers after updated sun position calculation #68

Merged
merged 3 commits into from
Jan 10, 2024

Conversation

jzuhone
Copy link
Member

@jzuhone jzuhone commented Jan 9, 2024

Description

This PR changes all "namespace" package imports to their new names. It also contains a follow-on to sot/ska_sun#36 which changed the default method of changing the sun position calculation, making it more accurate. This necessitated an update to the answers for the regression tests, which only had minor changes due to this update.

Interface impacts

None

Testing

Unit tests

  • No unit tests
  • Mac
  • Linux
  • Windows

Independent check of unit tests by [REVIEWER NAME]

  • [PLATFORM]:

Functional tests

No functional testing.

Copy link
Contributor

@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. Namespace package updates are good. I spot-checked the "answers" diffs for one states file and see expected diffs: small changes in pitch and changes in maneuver attitude quaternions at the 12th decimal place. The latter is likely from using numba not numpy, but in any case is inconsequential.

@jeanconn
Copy link

jeanconn commented Jan 10, 2024

The updates look good to me. I'd like to run the unit and regression tests but is there a recommended way to do so without installing the package? If I try to run pytest or python setup.py test from the repo I just get those E AttributeError: 'Namespace' object has no attribute 'test_root' errors.

@jzuhone
Copy link
Member Author

jzuhone commented Jan 10, 2024

@jeanconn from the top-level directory run py.test acis_thermal_check/tests/dpa for the 1DPAMZT tests

Copy link
Contributor

@Gregg140 Gregg140 left a comment

Choose a reason for hiding this comment

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

Namespace changes look good

@jzuhone jzuhone merged commit bec69d0 into master Jan 10, 2024
1 check passed
@javierggt javierggt mentioned this pull request Jan 11, 2024
@javierggt javierggt mentioned this pull request Feb 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.

4 participants