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

Update Truth: For dtcenter/MET#2960 #2673

Open
6 of 11 tasks
JohnHalleyGotway opened this issue Sep 4, 2024 · 3 comments · Fixed by #2683
Open
6 of 11 tasks

Update Truth: For dtcenter/MET#2960 #2673

JohnHalleyGotway opened this issue Sep 4, 2024 · 3 comments · Fixed by #2683
Assignees
Labels
alert: NEED ACCOUNT KEY Need to assign an account key to this issue component: CI/CD Continuous integration and deployment issues priority: blocker Blocker requestor: METplus Team METplus Development Team type: update truth Update truth dataset

Comments

@JohnHalleyGotway
Copy link
Collaborator

Describe Expected Changes

This issue and PR fixes contingency table statistics bugs that result in small but significant changes to 3 CTS line type output columns: SEDI_NCL, SEDI_NCU, and BAGSS. Differences are expected in all of these CTS columns, but nowhere else. Of course, if these values are requested in Series-Analysis config files, we should see diffs there as well.

Define the Metadata

Title

  • Define the Title of this issue as Update Truth: For dtcenter/{REPO}#{PR_NUMBER} to indicate the repository and pull request that warranted this issue.

Assignee

Assign this issue to the author of the pull request that warranted this issue. Optionally assign anyone else who should review the differences in the output.

  • Select engineer(s) or no engineer required:
    • Assigning @JohnHalleyGotway since he fixed the bug and @jprestop since she reviewed it and is familiar with the expected differences.
  • Select scientist(s) or no scientist required

Milestone and Projects

  • Select Milestone as the next official version if updating truth data for the develop branch OR select next METplus-Wrappers-X.Y.Z bugfix version if updating truth data for a main_vX.Y branch.
  • If updating truth data for the develop branch, select the METplus-Wrappers-X.Y.Z Development project OR if updating truth for a main_vX.Y branch, select the Coordinated METplus-X.Y Support project

Update Truth Checklist

  • Review the GitHub Actions workflow that was triggered by the PR merge
    • If no differences were found, note this in a comment.
    • If all of the differences are expected, note this in a comment.
      Include any details of how the review was performed.
    • If unexpected differences are found, the following instructions can
      help uncover potential explanations. If none of these apply and the
      source of the differences cannot be determined, contact the
      METplus wrappers lead engineer (@georgemccabe) for assistance.
      • Search for other open issues that have the label type: update truth
        applied by clicking on the label on this issue. Coordinate with the
        author of these issues to ensure all diffs are properly reviewed.
      • Check if any additional GitHub Actions testing workflows have been
        triggered since the workflow that corresponds to this issue was run.
        Review the latest run to ensure that there are no diffs that are
        unrelated to this issue.
      • If the incorrect differences are caused by the changes from the
        issue that warranted this issue, consider reverting the PR and
        re-opening the issue.
    • Iterate until one of the above conditions apply.
  • Approve the update of the truth data
    • Contact the METplus wrappers lead engineer (@georgemccabe) or
      backup lead (@jprestop) to let them know that the truth data can
      be updated.
  • Update the truth data.
    This should be handled by a METplus wrappers engineer.
    See the instructions to update the truth data
    for more info.
  • Close this issue.
@JohnHalleyGotway JohnHalleyGotway added priority: blocker Blocker alert: NEED ACCOUNT KEY Need to assign an account key to this issue component: CI/CD Continuous integration and deployment issues requestor: METplus Team METplus Development Team type: update truth Update truth dataset labels Sep 4, 2024
@JohnHalleyGotway JohnHalleyGotway added this to the METplus-5.1.1 Bugfix milestone Sep 4, 2024
@jprestop
Copy link
Collaborator

jprestop commented Sep 5, 2024

Defer to @georgemccabe on whether or not this work will be completed by patching the main_v5.1 branch to allow this workflow to run.

@jprestop
Copy link
Collaborator

jprestop commented Sep 9, 2024

@georgemccabe I ran the workflow again, but unfortunately I received another error:

Run branch_name=main_v5.[1](https://github.com/dtcenter/METplus/actions/runs/10777672190/job/29887286282#step:4:1)
main_v5.1-ref does exist -- update it
git checkout main_v5.1
Already on 'main_v5.1'
Your branch is up to date with 'origin/main_v5.1'.
git checkout -b update_main_v5.1_3551ec03
Switched to a new branch 'update_main_v5.1_3551ec03'
git merge -s ours origin/main_v5.1-ref
Merge made by the 'ours' strategy.
git checkout origin/main_v5.1-ref -- .github/update_truth_change_log.txt
error: pathspec '.github/update_truth_change_log.txt' did not match any file(s) known to git
Error: Process completed with exit code 1.

I know you had made some changes, but I'm guessing this error is still unexpected.

Could you please take a look when you get a chance? Any help you can give would be much appreciated.

@georgemccabe
Copy link
Collaborator

Hi @jprestop, I just pushed a change that should fix this issue. I pass the command to get the changelog from the *-ref branch to always return success to prevent this failure when the changelog does not exist. Try this workflow again and let me know if it still does not work as expected.

@jprestop jprestop linked a pull request Sep 9, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
alert: NEED ACCOUNT KEY Need to assign an account key to this issue component: CI/CD Continuous integration and deployment issues priority: blocker Blocker requestor: METplus Team METplus Development Team type: update truth Update truth dataset
Projects
Development

Successfully merging a pull request may close this issue.

3 participants