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

Rename observation_id #101

Merged
merged 2 commits into from
Mar 2, 2021
Merged

Rename observation_id #101

merged 2 commits into from
Mar 2, 2021

Conversation

javierggt
Copy link
Contributor

@javierggt javierggt commented Mar 1, 2021

Description

This PR follows PR #86. In that PR, a unique identifier for observations (observation_id) was introduced to the mag-stats processing, from command-line arguments to the supplement tables. This renames it to mp_starcat_time.

Rebase and review after #92 is merged

Testing

  • Passes unit tests on MacOS, linux, Windows (at least one required)
  • Functional testing. I did the same test as in Added a mechanism to set mag_aca and mag_aca_err manually in the supplement, and a unique star-observation ID #86. I checked that the generated obs-status.yml file uses the new label and that the supplement ended up having the table with the new label.
    # set it to import supplement from here
    export AGASC_DIR=`pwd`
    cp $SKA/data/agasc/* .;
    
    # create a list of interesting stars to test
    echo 331751984 > agasc_id # an OK new star
    echo 991559968 >> agasc_id # star fails (anyway there should be a single-star page for this star)
    echo 505545160 >> agasc_id # only one suspect obs (anyway there should be a singla-star page for this star)
    echo 42864776 >> agasc_id # an OK older star that gets updated
    echo 1200760024 >> agasc_id  # one suspect OBS (the suspect OBS should show up in the single-star page)
    echo 314189096 >> agasc_id
    echo 553396016 >> agasc_id
    
    # start with a set of OK stars
    agasc-update-magnitudes --log-level debug --start 2011:151 --stop 2011:152 --report
    agasc-update-magnitudes --log-level debug --start 2019:280 --stop 2019:281 --report
    
    # add the interesting stars, check the report and check the resulting obs_status.yml
    agasc-update-magnitudes --agasc-id-file agasc_id --log-level debug --report
    open supplement_reports/weekly/latest/index.html
    
    # check obs_status.yml, which should include observations, including the observation ID (the date)
    
    # One could update the supplement 'obs' table with this...
    # agasc-update-supplement --obs-status-file obs_status.yml # and check supplement
    
    # but that would not update the existing report. We do the following instead
    # the important thing is that the report directory must be the same, otherwise it does not work
    # the report directory is given by the stop time (this still needs a better approach)
    agasc-update-magnitudes --obs-status-file obs_status.yml --log-level debug --report
    open supplement_reports/weekly/latest/index.html
    # and check supplement
    

Fixes #95

@javierggt javierggt requested a review from taldcroft March 1, 2021 19:21
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.

Sorry for forcing this detour...

@javierggt javierggt merged commit 0242be1 into master Mar 2, 2021
@javierggt javierggt deleted the rename-observation_id branch March 2, 2021 19:15
@javierggt javierggt mentioned this pull request Mar 2, 2021
javierggt added a commit that referenced this pull request Apr 5, 2021
@javierggt javierggt mentioned this pull request Apr 6, 2021
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.

Change observation_id label
2 participants