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

Minor fixes #103

Merged
merged 12 commits into from
Mar 8, 2021
Merged

Minor fixes #103

merged 12 commits into from
Mar 8, 2021

Conversation

javierggt
Copy link
Contributor

@javierggt javierggt commented Mar 1, 2021

Description

This PR introduces some small changes:

Testing

open supplement_reports/weekly/latest/index.html
Fixes #79
Fixes #87
Fixes #88
Fixes #93
Fixes #98
Fixes #99
Fixes #106
Fixes #108

@@ -881,7 +881,7 @@ def get_agasc_id_stats(agasc_id, obs_status_override=None, tstop=None):
stats['mean_corrected'] = np.nan
stats['weighted_mean'] = np.nan

star = get_star(agasc_id, date=all_telem['times'][0])
star = get_star(agasc_id)
Copy link
Member

Choose a reason for hiding this comment

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

Should this have use_supplement=False? Otherwise the MAG_ACA below will be from the supplement if available.

Copy link
Contributor

Choose a reason for hiding this comment

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

I thought we'd captured that as a separate issue but I guess I forgot to do so.

Copy link
Member

Choose a reason for hiding this comment

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

This is now #106

@taldcroft
Copy link
Member

I looked through each commit and they all look reasonable. Thanks for the atomic commits and helpful commit messages. For 4e98da8, I couldn't easily see what this is doing without opening the code, but if functional testing shows that it closes #87 then 👍 . So this probably just needs basic functional testing.

…mined from the intersection of aca_l0.slot_data and the times of AOACMAG in the MSID set times (as opposed to fetching just AOACMAG to determine time). Fixes #108
Copy link
Member

@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.

When you think it's ready.

@javierggt javierggt merged commit fb13ebc into master Mar 8, 2021
@javierggt javierggt deleted the minor-fixes branch March 8, 2021 20:47
@javierggt javierggt mentioned this pull request Jul 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment