Skip to content

align: trim down print statements for insertions #1737

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.

Sign up for GitHub

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

Jump to bottom
Closed
joverlee521 opened this issue Jan 22, 2025 · 2 comments · Fixed by #1772
Closed

align: trim down print statements for insertions #1737

joverlee521 opened this issue Jan 22, 2025 · 2 comments · Fixed by #1772
Labels
proposal Proposals that warrant further discussion

Comments

@joverlee521
Copy link
Contributor

Context

I noticed that augur align prints insertions to stdout, which can potentially lead to workflow logs being flooded with long insertion sequences as noted in nextstrain/hmpv#8. I think we can trim down these print statements since the same insertions are output to a file. It's not entirely clear to me from reading #449 why insertions need to be print to stdout in addition to the output CSV file.

Possible solution

  1. Remove the print statements entirely
  2. Only print to stdout for debugging i.e. when the command is run with the --debug flag
  3. Keep the print statements because of (?) and always redirect stdout from augur align to a log file in workflows.
@joverlee521 joverlee521 added the proposal Proposals that warrant further discussion label Jan 22, 2025
@joverlee521 joverlee521 mentioned this issue Jan 22, 2025
Open
@jameshadfield
Copy link
Member

That was a long time ago! No reason for them to go to stdout as far as I can see. Option 1.

joverlee521 added a commit that referenced this issue Mar 13, 2025
@joverlee521
align: Remove insertion sequence print statements
20ee9d9 
These print statements can flood the workflow logs with long insertion
sequences that make it hard to parse. The same insertions are already
output to the `insertion_csv` so there's no reason they need to be
printed to stdout.

Note I opted to keep the warning logs for an insertion of gaps since
that is not captured in the `insertion_csv`.

Resolves <#1737>
@joverlee521 joverlee521 mentioned this issue Mar 13, 2025
Merged
4 tasks
@joverlee521
Copy link
Contributor Author

I ran into the flooded logs again this morning looking at the failed hmpv workflow so I'm just removing the logs in #1772.

joverlee521 added a commit that referenced this issue Apr 3, 2025
@joverlee521
align: Remove insertion sequence print statements
1516ab4 
These print statements can flood the workflow logs with long insertion
sequences that make it hard to parse. The same insertions are already
output to the `insertion_csv` so there's no reason they need to be
printed to stdout.

Note I opted to keep the warning logs for an insertion of gaps since
that is not captured in the `insertion_csv`.

Resolves <#1737>
joverlee521 added a commit that referenced this issue Apr 8, 2025
@joverlee521
align: Remove insertion sequence print statements
859abff 
These print statements can flood the workflow logs with long insertion
sequences that make it hard to parse. The same insertions are already
output to the `insertion_csv` so there's no reason they need to be
printed to stdout.

Note I opted to keep the warning logs for an insertion of gaps since
that is not captured in the `insertion_csv`.

Resolves <#1737>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
proposal Proposals that warrant further discussion
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

align: Remove insertion sequence print statements nextstrain/augur
2 participants
@jameshadfield @joverlee521