Set contents of temporary file if reference data does not match in ref_data_compare_from_paths.ipynb
#58
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
atharva-2001
force-pushed
the
raise_error_refdata_compare
branch
from
37bee85 to
77376d5
Compare
atharva-2001
changed the title
ref_data_compare_from_paths.ipynbref_data_compare_from_paths.ipynb
6 tasks
atharva-2001
force-pushed
the
raise_error_refdata_compare
branch
from
e58fbcb to
662a482
Compare
chvogl
approved these changes
Oct 27, 2022
wkerzendorf
reviewed
Oct 27, 2022
| "source": [ | ||
| "if False in tt.match.values or None in tt.match.values:\n", | ||
| " print(\"Reference files do not match, setting contents of tmp file to zero.\")\n", | ||
| " with open('/tmp/refdata_compare_result', 'w+') as file:\n", |
There was a problem hiding this comment.
atharva-2001
added a commit
to atharva-2001/carsus
that referenced
this pull request
Oct 30, 2022
atharva-2001
force-pushed
the
raise_error_refdata_compare
branch
2 times, most recently
from
9e50035 to
662a482
Compare
atharva-2001
force-pushed
the
raise_error_refdata_compare
branch
from
138c75f to
e2ecfdc
Compare
wkerzendorf
reviewed
Nov 10, 2022
| "if False in tt.match.values or None in tt.match.values:\n", | ||
| " print(\"Reference files do not match, setting contents of tmp file to zero.\")\n", | ||
| " with open('refdata_compare_result', 'w+') as file:\n", | ||
| " file.write('REFDATA COMPARISON FAILED')\n", |
There was a problem hiding this comment.
don't use file it's a reserved word in python fh
afloers
approved these changes
Nov 17, 2022
sonachitchyan
pushed a commit
that referenced
this pull request
Dec 6, 2022
…ef_data_compare_from_paths.ipynb` (#58) * Set contents of tmp file to zero if datasets do not match * change file location and contents * Rename file variable to fh
atharva-2001
added a commit
to tardis-sn/carsus
that referenced
this pull request
Feb 22, 2023
* Initial commit change env file path change env file path change path again change path again skip first step path again Package path * Add step to download the CMFGEN database * Add URLs to the CMFGEN database Fix atom data path * Add tardis build job repo path test * Add strategy for TARDIS build job * Fix lock file path Fix package path * Add step to run notebook * Upload spectrum notebook Fix notebook path Fix notebook path Require carsus build to run again before running tardis build * Fix atom data path Fix output format of atom data notebook Fix atom data path again Print atom data path Fix atom data path * Change URL used to get the reference data file * Download the file from the workflow * Upload reference data as artifact * Try downloading all reference data * Clone my fork of tardis refdata * Use lfs when cloning * Separate commands * Add new env variables, generate reference data for comparision using the new atomic data file * Clone my fork with arraydiff unpinned Fix atomic data path * Fix notebook path when creating artifact * Fix paths in environment variables * Install bokeh * Fix paths after moving branch * Rename notebook * Remove variables related to matrix strategy * Remove step to download reference data * Create copy of the quickstart notebook * Delete markdown * Add refdata_gen notebook instead of quickstart * Change branch when cloning carsus in the workflow * Try removing chianti reader * Put back elements in the atomic data * Add chianti data back but without collisions * Change Chianti version * Continue workflow if step fails * Revert back Chianti version * Add step to fail job if spectra dont match * test * Exit if contents of tmp file are set to 0 * Exit code * revert comments * On workflow dispatch * Rename steps and use variables * Replace NBCONVERT_CMD variable with NBCONVERT_FLAGS * Comment out Carsus build job temporarily, download reference data from ci helpers * Disable workflow dispatch temporarily * Fix paths for refdata compare notebook * Carsus build is necessary for TARDIS build * Change file name in notebook * Delete lines commented out which are not necessary * Do not clone my fork of the carsus repository * Change title of the notebook and rename it * Change file and step names * No need for inputs * Check file contents according to changes in tardis-sn/tardis-refdata#58 * Change path of refdata_compare_result file * Change trigger back to workflow_dispatch * Update URL to use TARDIS fork * Do not pin bokeh * Add name to comparison notebook artifact
andrewfullard
pushed a commit
to tardis-sn/carsus
that referenced
this pull request
Mar 30, 2023
…327) * Initial commit change env file path change env file path change path again change path again skip first step path again Package path * Add step to download the CMFGEN database * Add URLs to the CMFGEN database Fix atom data path * Add tardis build job repo path test * Add strategy for TARDIS build job * Fix lock file path Fix package path * Add step to run notebook * Upload spectrum notebook Fix notebook path Fix notebook path Require carsus build to run again before running tardis build * Fix atom data path Fix output format of atom data notebook Fix atom data path again Print atom data path Fix atom data path * Change URL used to get the reference data file * Download the file from the workflow * Upload reference data as artifact * Try downloading all reference data * Clone my fork of tardis refdata * Use lfs when cloning * Separate commands * Add new env variables, generate reference data for comparision using the new atomic data file * Clone my fork with arraydiff unpinned Fix atomic data path * Fix notebook path when creating artifact * Fix paths in environment variables * Install bokeh * Fix paths after moving branch * Rename notebook * Remove variables related to matrix strategy * Remove step to download reference data * Create copy of the quickstart notebook * Delete markdown * Add refdata_gen notebook instead of quickstart * Change branch when cloning carsus in the workflow * Try removing chianti reader * Put back elements in the atomic data * Add chianti data back but without collisions * Change Chianti version * Continue workflow if step fails * Revert back Chianti version * Add step to fail job if spectra dont match * test * Exit if contents of tmp file are set to 0 * Exit code * revert comments * On workflow dispatch * Rename steps and use variables * Replace NBCONVERT_CMD variable with NBCONVERT_FLAGS * Comment out Carsus build job temporarily, download reference data from ci helpers * Disable workflow dispatch temporarily * Fix paths for refdata compare notebook * Carsus build is necessary for TARDIS build * Change file name in notebook * Delete lines commented out which are not necessary * Do not clone my fork of the carsus repository * Change title of the notebook and rename it * Change file and step names * No need for inputs * Check file contents according to changes in tardis-sn/tardis-refdata#58 * Change path of refdata_compare_result file * Change trigger back to workflow_dispatch * Update URL to use TARDIS fork * Do not pin bokeh * Add name to comparison notebook artifact * Cache LFS objects * Do git lfs checkout if the cache key is found * Use cache no instead of v1 hardcoding
andrewfullard
pushed a commit
to tardis-sn/carsus
that referenced
this pull request
Mar 30, 2023
* Initial commit change env file path change env file path change path again change path again skip first step path again Package path * Add step to download the CMFGEN database * Add URLs to the CMFGEN database Fix atom data path * Add tardis build job repo path test * Add strategy for TARDIS build job * Fix lock file path Fix package path * Add step to run notebook * Upload spectrum notebook Fix notebook path Fix notebook path Require carsus build to run again before running tardis build * Fix atom data path Fix output format of atom data notebook Fix atom data path again Print atom data path Fix atom data path * Change URL used to get the reference data file * Download the file from the workflow * Upload reference data as artifact * Try downloading all reference data * Clone my fork of tardis refdata * Use lfs when cloning * Separate commands * Add new env variables, generate reference data for comparision using the new atomic data file * Clone my fork with arraydiff unpinned Fix atomic data path * Fix notebook path when creating artifact * Fix paths in environment variables * Install bokeh * Fix paths after moving branch * Rename notebook * Remove variables related to matrix strategy * Remove step to download reference data * Create copy of the quickstart notebook * Delete markdown * Add refdata_gen notebook instead of quickstart * Change branch when cloning carsus in the workflow * Try removing chianti reader * Put back elements in the atomic data * Add chianti data back but without collisions * Change Chianti version * Continue workflow if step fails * Revert back Chianti version * Add step to fail job if spectra dont match * test * Exit if contents of tmp file are set to 0 * Exit code * revert comments * On workflow dispatch * Rename steps and use variables * Replace NBCONVERT_CMD variable with NBCONVERT_FLAGS * Comment out Carsus build job temporarily, download reference data from ci helpers * Disable workflow dispatch temporarily * Fix paths for refdata compare notebook * Carsus build is necessary for TARDIS build * Change file name in notebook * Delete lines commented out which are not necessary * Do not clone my fork of the carsus repository * Change title of the notebook and rename it * Change file and step names * No need for inputs * Check file contents according to changes in tardis-sn/tardis-refdata#58 * Change path of refdata_compare_result file * Change trigger back to workflow_dispatch * Update URL to use TARDIS fork * Initial commit * Do not pin bokeh * Download atom data, prevent the bridge from failing temporarily * Zip artifacts into one * Rename paths and artifact name * Rename jobs * Send file via SCP instead of saving it as an artifact * Add commit hash to file name * Continue saving the files even if the bridge fails * Try using if:always() * Uncomment cron job code * Trigger zip_artifacts job only if the last workflow's last step was triggered * Rename jobs and workflows: don't use underscore * Uncomment the cron job code * workflow dispatch trigger to test manually * croj job: change 7 to 0
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
📝 Description
Type: 🪲
bugfixThis PR changes the
ref_data_compare_from_paths.ipynbnotebook to set the contents of a file in the/tmpdirectory to zero if the datasets do not match. The bridge workflow(tardis-sn/carsus#313) then checks the contents of this file and fails if zero is present.An alternative to this could have been raising an error if the datasets failed to match- but nbconvert didn't allow converting notebooks to HTML if they had errors, which is why we had to go with this.
📌 Resources
Examples, notebooks, and links to useful references.
🚦 Testing
How did you test these changes?
☑️ Checklist
build_docslabel