-
Notifications
You must be signed in to change notification settings - Fork 23
Assign Use to AU workflow in ExampleMod2 vignette #645
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
Assign Use to AU workflow in ExampleMod2 vignette #645
Conversation
Made some small edits to the workflow to reflect uses to new AU example. Modified the code to include ATTAINS.WaterType in the workflow for TADA_CreateAUMLCrosswalk workflow in the bathchupload dataframe list. @hillarymarler if you can please review to make sure the changes work fine and don't interfere with any other workflows that would be great!
Yes - I can review this. Thanks for all the updates |
When a match is not made between a WQP Monitoring Location and an AU from the specified org, should that row be excluded from the df when batch_upload = TRUE? I am not sure if we should add a filter to remove when AU = NA when batch_upload = TRUE as those records can't be stored in ATTAINS (and may generate an error if the user attempts to upload) because MLs can only be saved in relation to an AU. What do you think @wokenny13 and @cristinamullin? The MT example in the ExampleMod2Worfklow provides an example of this where not all MLs are matched with AUs by the end of the workflow. |
vignettes/ExampleMod2Workflow.Rmd
Outdated
|
||
For any new AU, users will need to determine how to assign uses to them. | ||
|
||
1) TADA has developed a function called TADA_CreateWaterUseRef which imports |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addition of WaterType here looks great.
existing uses by water type from ExpertQuery, and will assign any new AU these uses. | ||
Users may choose to edit the TADA_CreateWaterUseRef according to their preference. | ||
|
||
```{r assign.use.to.new.AU.with.waterUseRef} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding the 2nd hash fixed the error for me. I will commit that change.
This is a good point & the approach you've laid out makes sense to me (assuming they cannot include MLs in ATTAINS that are not matched with AUs). But what if an entity wants to track WQP monitoring locations that they have reviewed and determined are not applicable to their analysis (AUs)? They might not want to have to review those locations again in the future and could include a note about why they are being excluded. Would they have to keep that part of the process outside of ATTAINS? If it cannot be included, I wonder if this could deter folks from using ATTAINS as the source of the ML/AU crosswalk... |
That's a good point. I wonder if it might be possible to create a generic "Unassigned" AU in ATTAINS for an org so that those MLs without matches could be recorded in ATTAINS. I can bring it up w/ the ATTAINS team. Then we might have to figure out a workaround for if/when we get to the point of users being able to add new AU geometry as they may want to check those previously unassigned MLs against the new AU(s). |
This was something that I was also wondering while working through this. On one hand, I think it provides a good summary of ML sites that could not be matched to any AU and allows users to decide whether they should be matched to any AUs or not if they want it as part of their assessment/analysis. But if they were to try to submit it as a batch upload, they can run into that problem with what you mentioned. I see benefits of including them as well as excluding them. I don't know if it would be useful to have some sort of flagging column to assist them in determining if it has been reviewed or not, and then give users the option to include or exclude them for compatibility in ATTAINS batch upload format. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than the weird error I'm seeing in the ExampleMod2Worfklow vignette all of these changes look good. Have you seen that error? I tried running through the whole workflow a couple of times and was seeing the same error consistently.
As far as the batch upload questions, I think we can close this PR without making changes/adding additional filters and can make any of those changes later after a discussion w/ the ATTAINS team.
Made some small edits to the workflow to reflect uses to new AU example.
Modified the code to include ATTAINS.WaterType in the workflow for TADA_CreateAUMLCrosswalk workflow in the bathchupload dataframe list. @hillarymarler if you can please review to make sure the changes work fine and don't interfere with any other workflows that would be great!