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

overwrite duplicate diagnostics #2713

Merged
merged 1 commit into from
Feb 27, 2024
Merged

overwrite duplicate diagnostics #2713

merged 1 commit into from
Feb 27, 2024

Conversation

juliasloan25
Copy link
Member

Purpose

Instead of raising an error when a user tries to add a diagnostic that already exists, this PR warns the user and overwrites the existing diagnostic with the new entry.

closes #2709

@juliasloan25 juliasloan25 self-assigned this Feb 23, 2024
@Sbozzolo
Copy link
Member

Maybe print out more information on what was there before overwriting?

@juliasloan25
Copy link
Member Author

Maybe print out more information on what was there before overwriting?

Do you want me to also show what it's getting overwritten with?

@juliasloan25 juliasloan25 changed the title overwrite duplicate diags overwrite duplicate diagnostics Feb 23, 2024
@juliasloan25 juliasloan25 force-pushed the js/diags branch 6 times, most recently from b6f057e to d515f69 Compare February 23, 2024 21:02
@Sbozzolo
Copy link
Member

Maybe print out more information on what was there before overwriting?

Do you want me to also show what it's getting overwritten with?

No, we assume that users will know what they called.

@juliasloan25
Copy link
Member Author

Okay! I updated the warning message so it will print something like

┌ Warning: overwriting diagnostic `mse` entry containing fields
│ [mse, Moist static energy, moist_static_energy, J/kg, Moist static energy]
└ @ ClimaAtmos.Diagnostics ~/.julia/packages/ClimaAtmos/gIAbX/src/diagnostics/diagnostic.jl:155

@juliasloan25 juliasloan25 force-pushed the js/diags branch 5 times, most recently from 73bce11 to e014e55 Compare February 23, 2024 23:15
@juliasloan25 juliasloan25 added this pull request to the merge queue Feb 24, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 24, 2024
@juliasloan25 juliasloan25 added this pull request to the merge queue Feb 24, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 24, 2024
@Sbozzolo Sbozzolo added this pull request to the merge queue Feb 26, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 26, 2024
@juliasloan25 juliasloan25 added this pull request to the merge queue Feb 27, 2024
Merged via the queue into main with commit 6e42940 Feb 27, 2024
9 of 11 checks passed
@juliasloan25 juliasloan25 deleted the js/diags branch February 27, 2024 23:01
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.

re-running add_diagnostic_variable! produces error (in coupler)
2 participants