597 - add opdtable as positional param to monthly_trending_plot

[kulpster85]

[codecov[bot]]

Codecov Report

Base: 56.41% // Head: 56.13% // Decreases project coverage by -0.27% :warning:

Coverage data is based on head (2510c7d) compared to base (cc16c90). Patch coverage: 14.28% of modified lines in pull request are covered.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #600      +/-   ##
===========================================
- Coverage    56.41%   56.13%   -0.28%     
===========================================
  Files           13       13              
  Lines         5871     5905      +34     
===========================================
+ Hits          3312     3315       +3     
- Misses        2559     2590      +31     

Impacted Files	Coverage Δ
webbpsf/trending.py	5.79% <10.00%> (-0.08%)	:arrow_down:
webbpsf/mast_wss.py	9.82% <18.18%> (+0.47%)	:arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[kulpster85]

I can add that logic to the changes - that's a good suggestion for robustness.

[kulpster85]

This looks good. Just adding the option for the user to provide the OPD table. However, how do you check that the table provide by the user has the same fields as those created by get_opdtable_for_month? e.g.,i'is_pre_correction' , 'is_post_correction' etc. which are fields that the monthly trending plot function and get_corrections are expecting to find. Maybe a check that all the keys in this OPD table are the same as the expectations.

I have pushed some commits to address your comments @obi-wan76

[mperrin]

@obi-wan76 can you re-review or approve this after the changes by @kulpster85 in response to your prior review? (Like a month ago... I think this is ready to merge but will defer to you to re-review)

[mperrin]

This is an old PR, from November, which it looks like @kulpster85 is still waiting for a review from @obi-wan76 for. Trey is this still a relevant/desired PR? If so, reminder to Marcio. :-)

[kulpster85]

This would be an efficiency improvement that TAS could leverage to provide the opdtable as an input rather than download the OPDs from MAST.

[mperrin]

@obi-wan76 FYI looks like you approved this back in March but never merged it...