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 allowed_rolldev to use chandra_models #27

Merged
merged 4 commits into from
Mar 1, 2023

Conversation

taldcroft
Copy link
Member

@taldcroft taldcroft commented Mar 1, 2023

Description

Use the pitch-roll constraint CSV data file in chandra_models via the interface in ska_helpers.chandra_models to import the table of pitch vs. off-nominal roll data.

Requires

Fixes #26

Interface impacts

No API changes, but smaller available available pitch/roll space available.

Testing

Unit tests

  • Mac

Independent check of unit tests by Jean

  • Linux ska3-prime 2023.1rc9

Functional tests

No functional testing.

@taldcroft taldcroft requested a review from jeanconn March 1, 2023 01:09
@jeanconn
Copy link
Contributor

jeanconn commented Mar 1, 2023

I put the updated ska_helpers in PYTHONPATH, updated my $SKA/data/chandra_models to the pitch-roll-constraint branch, and ran tests here and get failed tests that chandra_models isn't at 3.47.

FAILED ska_sun/tests/test_sun.py::test_allowed_rolldev[0.0--1.0] - ValueError: repo tip is not at tag 3.47

This is on linux. Not sure what I'm missing just yet.

@jeanconn
Copy link
Contributor

jeanconn commented Mar 1, 2023

Overall the "repo tip is not at tag" thing keeps biting me. More a ska_helpers issue (if the xija code is going in there) than ska_sun but I could use a "what are we checking and why do we care" overall on this. Did I do something wrong? How is one supposed to work with a test chandra_models? Set some other flag? Make a draft tag?

@jskrist
Copy link
Member

jskrist commented Mar 1, 2023

@jeanconn, I also bumped on the repo tip is not at tag thing. I think the intention is to make sure that the user is getting the data they expect (typically the latest data), but I wonder if there is a way to check if the data file to be read has any modifications, as local modifications would also cause potentially unexpected data to be returned.

perhaps this should be a log message or warning instead of an exception, or have some way for a power user/dev to skip this check/exception when working in a development environment?

@jeanconn
Copy link
Contributor

jeanconn commented Mar 1, 2023

@jskrist it looks like it will stop complaining if I add a tag in the testing commit in chandra_models, which is probably a fine work around for the power-testing user.

@taldcroft taldcroft merged commit e43f1e1 into master Mar 1, 2023
@taldcroft taldcroft deleted the pitch-roll-constraint branch March 1, 2023 19:07
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 pitch roll constraint package data and use chandra_models
3 participants