-
Notifications
You must be signed in to change notification settings - Fork 9
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
Duplicate MUSICA name keys are not detected #235
Comments
Oh, yes, sorry, that was my mistake. It’s a good idea to add an alert to remind users. Thanks for that! |
The run-time error looks like this: 2024-09-12 13:28:52 - DEBUG - main.main - Working directory = /home/drews/MusicBox/music-box-duplicate-keys-235/temp |
The branch is here: Not completed yet - still need to add the unit test. |
Pull request is: |
…235 (#241) * Adding check for duplicate MUSICA keys. * Removed diagnosic code. * Added stub for unit test. * Added the Pass test config. * Finalized the test for duplicate reaction names. * Moved from integration tests to unit tests. * Use pytest to verify exception raised. * Removed unneeded imports. * Simpler assert check for success. * Moved the configuration check to within readConditionsFromJson(), and at the end. * Fixed indentation mistake. --------- Co-authored-by: carl-drews <drews@modeling2.acom.ucar.edu>
@SancyW reported a configuration to use which was failing in music box interactive. The configuration is listed below. The issue is that there were 6 reactions which were actually 3 reactions, each duplicated. Their musica names are
configuration.zip
Reproduce
To see the incorrect behavior, download and unzip the configuraiton and run
music_box -c configuration/my_config.json -o output.csv
Observe that there is a key index error.
Then, open up
configuration/camp_data/reactions.json
and find the duplicated reactions above. Delete one of each pair and rerun the configuration. You'll see that it passes.We should alert when we find duplicate keys and let the user know and then exit
Acceptance criteria
Ideas
The text was updated successfully, but these errors were encountered: