Remove PARAM_MAP, use ReefGuide.ASSESSMENT_CRITERIA (for LowHighTide) #3
Conversation
This file contains hidden or 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
…rs to be more strict, only include criteria in user params
arlowhite
force-pushed
the
low-high-criteria
branch
from
c13ea42 to
6b6cf23
Compare
arlowhite
commented
Aug 7, 2025
src/job_worker/handler_helpers.jl
Outdated
| ) | ||
|
|
||
| @debug "Merged bounds" min_val = bounds.min max_val = bounds.max user_specified_min = | ||
| @debug "Merged bounds for $(criteria.metadata.id)" min_val = bounds.min max_val = |
Collaborator
Author
There was a problem hiding this comment.
kinda ugly, JuliaFormatter did this.
is there a multiline break syntax I could use?
Collaborator
There was a problem hiding this comment.
Yeah, I'm not a fan of this sequential assignment method either - I think it's hard to read.
I prefer to use multiline strings:
@debug """
Merged bounds for $(criteria.metadata.id):
min_val: $(bounds.min)
max_val: $(bounds.max)
...
"""I think the above should work.
arlowhite
changed the title
ConnectedSystems
approved these changes
Aug 7, 2025
Collaborator
ConnectedSystems
left a comment
ConnectedSystems
left a comment
There was a problem hiding this comment.
Very minor comments.
PeterBaker0
approved these changes
Aug 11, 2025
Collaborator
PeterBaker0
left a comment
PeterBaker0
left a comment
There was a problem hiding this comment.
Happy with this - thanks - it's cleaner
|
|
||
| # NOTE: you can perform additional setup here if needed. For example, you | ||
| # might want to initialise data, caches or clients. | ||
| # might want to initialize data, caches or clients. |
Collaborator
There was a problem hiding this comment.
lol - we prefer american spelling? Is this a US agenda Arlo?
improved comments Co-authored-by: Takuya Iwanaga <[email protected]>
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.
open-AIMS/reefguide#74
PARAM_MAP, useReefGuide.ASSESSMENT_CRITERIAinsteadRegionalAssessmentInputandSuitabilityAssessmentInputwith new criteria, alphabetical sortThe major change in this PR is that
build_regional_assessment_parametersbehavior changed.Previously, it would ignore missing data and set default bounds even when a criteria is not given. Now, it errors when criteria data is missing, and only sets a criteria param if at least one of the min|max params is specified.