Require Options to be exactly one of either a section or a value
#2277
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
Potentially breaking change, as previously it was possible to have an Options be both a section and a value
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
|
|
Hmm, I've just double-checked that and it doesn't throw an error -- ah, you mean in the case-sensitive I just quickly tested what happens when this gets merged into #2210, and it looks like it needs a bit more work to play nice with this change. It might actually need to throw an error in this PR if something that was a value is used as a section. This PR just silently converts between values and sections, which on reflection is not a good idea! |
|
Sorry, yes I meant with #2210 I thought it would be (should be!) an error. |
Only applies to non-empty sections
|
Ok, this now catches |
* next: Remove deprecated MsgStack methods Fix for use of deprecated std::uncaught_exception CMake: Fix netCDFCxx version parsing Update cmake/FindnetCDFCxx.cmake CMake: Find the Libuuid dependency CMake: Check if _ncxx4_version is set Fix segfault when creating LaplaceXZ without passing Options Bump jinja2 from 2.10.1 to 2.11.3 remove LAM add cd to instruction Restrict region for toFieldAligned() in VDDY() when not staggered Update tests/integrated/test-beuler/CMakeLists.txt Add test for backward Euler Initialise some variables in SNES/beuler solver Silence clang-tidy warnings on PETSc CHKERRQ Ignore "beuler" Solver in test Improve handling of errors Clang tidy suggestions Add Backward Euler time integrator
- 'restart' section renamed to 'restart_files' - 'input' value already a library input section
* next: (28 commits) Move constraint creation to utility method CMake: Default to not building the docs CMake: Be explicit that docs aren't built in `all` CMake: Fix typo Make method parameter names consistent (clang-tidy fix) Check version of SUNDIALS is new enough for constraints Option to set positivity constraints in CVODE Option to change max nonlinear iterations in CVODE GHA: Don't build the docs for other Github actions either GHA: Don't build docs on github CMake: Build documentation [skip ci] Apply black changes Update installation suggestions for Marconi/gnu fix order of date and time Fix compilation Faster recompile Remove checks for old behaviour of bool conversion Use lowercase() from utils.hxx Make conversion of Option to bool stricter Add brackets to previous commit ...
* next: (60 commits) CMake: Add 'docs' synonym for sphinx-html target CMake: Add PYTHONPATH env var to docs command CMake: Use correct latex builder invocation CMake: Fix missing 'echo' in docs command Docs: Fix use of deprecated/removed Sphinx method Remove extraneous `this->` when setting member variables Fix a couple of typos in tests Only check local y points if we have multiple processors in Y Docs: Fix sphinx conf.py for breathe 4.30 CMake: Print link to generated docs after completion CMake: Fix typo in docs comment [skip ci] Apply black changes Print possible boundary regions during startup Add Mesh method to get the set of all possible boundaries Avoid warning about null `this` in GCC11 Fix initialisation of MPI types Use same argument name in declaration and definition Use default initialisation of member Immediately initialise some local variables Use vector::empty rather than compare size to zero ...
Fixed conflicts in: CHANGELOG.md bin/bout-v5-input-file-upgrader.py include/options.hxx src/sys/options.cxx * next: (237 commits) CMake: Fail fast in CI script CMake: Add REQUIRES, CONFLICTS arguments to bout_add_model/example Fix permissions Add simple test for bout-config CMake: Fix issue on Fedora with PETSc linking order with bout-config CMake: Use less jargon in function docstring CMake: Fix correct library location for shared lib in bout-config CMake: Fix for fmt library name in debug builds Use better variable names Add downloading dependencies to CMake install doc CMake: Add option to download SUNDIALS at configure time Delete some commented out code Remove some unused private members Fix clang-tidy warning about uninitialised members Use override on overridden virtual functions Make some LaplacePCR methods const Reduce scope of some local variables and immediately initialise them Fix some clang-tidy warnings Fix use of reserved identifier for header guard Keep const in cxx ...
|
Had to fix the inevitable conflicts with #2210! |
johnomotani
approved these changes
May 28, 2021
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.
This is a breaking change, but I don't think it will affect very many people, and turns out to have basically no effect on the logic anywhere, but does automatically fix a small bug: when creating a new section, it would not be marked as a section:
Previously, both
section1andsection2would haveis_section == falseandis_value == false. This PR fixes both cases: anOptionsis a section until it has a value assigned to it, when it becomes a value.This does prevent us having an input file like the following:
but I think that's probably a good thing. I think
restartis the only case of this in the library, but of course users might have other instances.Renames the
restartsection torestart_files-- this doesn't appear in any test or example input files, so I suspect it's likely not used in the wild either, but the input file upgrader can fix it if it does.