Skip to content
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

Extending get_locs() #44

Merged
merged 30 commits into from
Mar 20, 2020
Merged

Extending get_locs() #44

merged 30 commits into from
Mar 20, 2020

Conversation

florisvdh
Copy link
Member

@florisvdh florisvdh commented Mar 6, 2020

@w-jan can you just have a look at the code changes?

This PR (for release candidate 0.3) adds more features to get_locs():

  • return extra observation well attributes (beside filterdepth): soilsurf_ost (soil surface in Ostend height CRS), tubelength, measuringref_ost (top of tube in Ostend height CRS), obswell_statecode, obswell_state, installdate, stopdate (the latter two are only kept if obswells = TRUE)
  • add logical arguments:
    • filterdepth_guess: try to fill missing filterdepth values with tubelength (as a conservative approach). This is done before applying the filterdepth_range condition
    • obswell_aggr: to allow for different methods of aggregating observation well attributes per location (3 methods of selecting one well, a fourth method provides the mean values)

With thanks to @w-jan for his ideas.

This is because in some cases records for the same observation well
exist with different statuscodes.
Accomplished by filtering for specific value of fields 'PeilpuntOpenbaarheidTypeCode' and 'PeilpuntOpenbaarheidCode'
This provides 4 approaches to aggregate obswell attributes to the location level, as discussed with @w-jan.
See function documentation.

Note: the use of window functions with dbplyr 1.4.2 generated a warning (here caused by row_number()), as
discussed and solved in tidyverse/dbplyr#328 (merged to dbplyr master).
As the fixed dbplyr master is currently not released, and watina depended on an (older) dbplyr fork with pivot_wider,
a merge between both is now required.
It is provided by https:/florisvdh/dbplyr/tree/dbplyr_with_pivot_wider
@florisvdh florisvdh requested a review from w-jan March 6, 2020 15:00
- measure filterdepth by the top of the filter
- (re)arrange obswells by area_code and loc_code
Copy link
Collaborator

@w-jan w-jan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks very good !
I made an adjustment for the calculation of the filterdepth and to rearrange the locs-table

@florisvdh
Copy link
Member Author

florisvdh commented Mar 16, 2020

Thanks @w-jan for the suggestions. I'll work this a bit further out as discussed previous week. Regarding filter depth however, I've searched around to evaluate what would be the best solution.

  • Nowhere I could find specifications to use the filter top to determine elevation head (= pressure head's bottom above sea level).
    • e.g. imagine that filter top would be exactly at the phreatic groundwater level. In a situation of groundwater discharge the measured hydraulic head would still be a bit higher than phreatic groundwater level because of the excess water pressure deeper below (along the filter).
  • Examples are available where the middle of the filter (= 'well screen') is used, and sometimes the bottom (images pasted below). So I actually plan to implement the middle, is that OK for you? @mathiaswackenier @cecileherr @ToonVanDaele @DriesAdriaens

image
(from slide 10 of this presentation)

image
(from here)

image
(from here)

image
(from here)

@florisvdh
Copy link
Member Author

Implemented a few enhancements. Waiting for responses on using filterlength/2 before implementing it to adjust both filterdepth and guessed filterdepth.

@florisvdh
Copy link
Member Author

Thanks @w-jan for discussing this with Piet De Becker. We will use filterlength / 2.

filterdepth was originally calculated using the filter bottom,
then it was altered (@w-jan) to use the filter top,
but eventually we settle using the middle of the filter (well screen).
@florisvdh
Copy link
Member Author

Going to merge this one for release candidate 0.3.0. Next a PR will open where more stuff could still be added in preparation of version 0.3.0.

You see, not everything shuts down these days ...

@florisvdh florisvdh merged commit c8b6ad1 into rc0.3 Mar 20, 2020
@florisvdh florisvdh deleted the get_locs branch March 20, 2020 13:25
@florisvdh florisvdh mentioned this pull request Mar 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants