Suna #1006
Suna #1006
Conversation
Merge branch 'master' into suna # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.
Merge branch 'master' into suna # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.
Merge branch 'master' into suna # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.
Merge branch 'master' into suna # Conflicts: # pipe/sunav2/sunav2_calibration_assignment.json # pipe/sunav2/sunav2_location_active_dates_assignment.json # pipe/sunav2/sunav2_location_asset_assignment.json # pipe/sunav2/sunav2_merge_data_by_location.json # pipe/sunav2/sunav2_structure_repo_by_location.json
Merge branch 'master' into suna # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.
…y input to the final quality flag.
…A rather than deleting line.
Merge branch 'master' into suna # Conflicts: # pipe/sunav2/sunav2_location_active_dates_assignment.json # pipe/sunav2/sunav2_location_asset_assignment.json
Merge branch 'master' into suna # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.
Merge branch 'master' into suna # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.
Reverting minor change from master
bring base image to current
bring base image to current
bring base image to current
bring base image to current
bring qaqc image to current
bring base image to current
bring cal image to current
bring image to current
bring image to current
covesturtevant
left a comment
covesturtevant
left a comment
There was a problem hiding this comment.
Overall looks great. A few moderate bits of work to address, but I don't think they'll be too much work. Please also create unit tests for new modules. Recommend feeding Copilot one of our unit tests as a template and asking it to create the ones for the new modules.
Please let me know if any comments could use clarification.
There was a problem hiding this comment.
Looks good, but please update the header to reflect that regularization is not performed. There is some conflicting language in a few places, likely from using flow.rglr as a template.
There was a problem hiding this comment.
Same comment as above - there remains documentation in the header that indicates regularization is performed.
There was a problem hiding this comment.
For security, please run as non-root user. See the Dockerfile for flow.gap.fill.nonrglr for these components.
| log=log | ||
| ), | ||
| error = function(err) { | ||
| call.stack <- base::sys.calls() # is like a traceback within "withCallingHandlers" |
There was a problem hiding this comment.
Why non-standard error handling?
| #' revert the insufficient data quality flag (default is to apply it). | ||
| qmData$insufficientDataQF=1 | ||
| minPoints<-as.numeric(minPoints) | ||
| for(i in 1:nrow(statsData)){ |
There was a problem hiding this comment.
vectorize this for-loop
| log=log | ||
| ), | ||
| error = function(err) { | ||
| call.stack <- base::sys.calls() # is like a traceback within "withCallingHandlers" |
There was a problem hiding this comment.
Why non-standard error-routing?
| humidityThreshold<-sunaThresholds[(sunaThresholds$threshold_name=="Nitrates Maximum Internal humidity"),] | ||
| maxHumidity<-humidityThreshold$number_value | ||
| sensorFlags$nitrateHumidityQF<-NA | ||
| for(i in 1:nrow(sunaData)){ |
There was a problem hiding this comment.
Please vectorize these for-loops throughout the code. When a simple, base-r vectorization exists, it is much preferable. Recommend putting into MS Copilot chat for help. For example, Copilot suggested the following to replace this block:
rh <- sunaData$relative_humidity
qf <- integer(length(rh)) # default 0
qf[is.na(rh)] <- -1L
qf[!is.na(rh) & rh > maxHumidity] <- 1L
sensorFlags$nitrateHumidityQF <- qf
There was a problem hiding this comment.
This change seems out of place. Intentional? Tested? If not, please revert.
There was a problem hiding this comment.
This does not look intentional. Please revert.
There was a problem hiding this comment.
Note for Cove to merge in the kafka loader to the main DAG
No description provided.