Skip to content
This repository has been archived by the owner on Oct 21, 2024. It is now read-only.

Factor orbit, FOV, and observing constraints into new Mission class #40

Merged
merged 1 commit into from
Apr 23, 2021

Conversation

lpsinger
Copy link
Contributor

I am not sure if this is the right abstraction, but at least it makes it so that the Dorado observing constraints are not in the one global visibility_constraints list.

@mcoughlin, could you see using this new Mission class?

@lpsinger lpsinger requested a review from mcoughlin April 20, 2021 03:10
@codecov
Copy link

codecov bot commented Apr 20, 2021

Codecov Report

Merging #40 (99340c1) into master (da6db2f) will increase coverage by 1.21%.
The diff coverage is 77.77%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #40      +/-   ##
==========================================
+ Coverage   23.52%   24.73%   +1.21%     
==========================================
  Files          25       26       +1     
  Lines        1424     1435      +11     
==========================================
+ Hits          335      355      +20     
+ Misses       1089     1080       -9     
Impacted Files Coverage Δ
dorado/scheduling/scripts/animate.py 8.25% <16.66%> (-0.44%) ⬇️
dorado/scheduling/scripts/main.py 9.61% <33.33%> (-0.39%) ⬇️
dorado/scheduling/mission.py 96.00% <96.00%> (ø)
dorado/scheduling/constraints/__init__.py 84.61% <100.00%> (-2.06%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update da6db2f...99340c1. Read the comment docs.

Copy link
Collaborator

@mcoughlin mcoughlin left a comment

Choose a reason for hiding this comment

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

Looks really nice. And I do like the python configurations.

We should also enable turning roll on or off perhaps? For now, I was solving with no roll.

I am not sure if this is the right abstraction, but at least it
makes it so that the Dorado observing constraints are not in the
one global visibility_constraints list.
@lpsinger
Copy link
Contributor Author

Looks really nice. And I do like the python configurations.

This doesn't use the Astropy configuration system. This just introduces a new class.

Can you see using this in the UVEX scripts?

We should also enable turning roll on or off perhaps? For now, I was solving with no roll.

For now, since Dorado hasn't decided whether or not to vary roll angles, I am thinking of the roll grid as a command line option. If you pass a roll step size of 90 degrees, that has the same effect as turning off the roll grid.

@mcoughlin
Copy link
Collaborator

Yes. I suppose just slight annoyance if you want to update mission constraints needing to modify repository, but I think it's fine.

@lpsinger
Copy link
Contributor Author

Yes. I suppose just slight annoyance if you want to update mission constraints needing to modify repository, but I think it's fine.

Fair point. I'll whip up an example of what the Astropy configuration system implementation would look like.

@lpsinger
Copy link
Contributor Author

I couldn't get the Astropy configuration system to work. I think it has to do with our module being called dorado.scheduling rather than dorado_scheduling. Can we adopt the Mission class as simply an incremental improvement?

@mcoughlin
Copy link
Collaborator

definitely better than nothing!

@lpsinger lpsinger merged commit 82b0cb6 into nasa:master Apr 23, 2021
@lpsinger lpsinger deleted the mission branch April 23, 2021 17:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants