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

Convert mdl units #57

Merged
merged 17 commits into from
Feb 19, 2021
Merged

Convert mdl units #57

merged 17 commits into from
Feb 19, 2021

Conversation

ateucher
Copy link
Contributor

No description provided.

@ateucher
Copy link
Contributor Author

Initial work to address bcgov/wqbc#158. Unfortunately it's not very fast (~11 seconds for 5000 rows) due to having to loop over the rows, since {units} isn't vectorized over different units.

@ateucher
Copy link
Contributor Author

@joethorley I think this is a pretty good solution, one TODO remaining.

@ateucher ateucher marked this pull request as ready for review February 18, 2021 23:50
@ateucher
Copy link
Contributor Author

@joethorley @JessicaPenno @KarHarker @j-krogh I am ready to merge this - please have a look if you'd like. See the README for a brief demo of its usage.

@joethorley
Copy link
Contributor

That is very elegant - hats off!

@ateucher
Copy link
Contributor Author

Thanks Joe!

@JessicaPenno
Copy link

Awesome Andy!! I assume that I do not need to review the units table anymore, I have not got to that yet

@joethorley how do we get this into ShinyREMs?

@ateucher
Copy link
Contributor Author

Thanks @JessicaPenno - no you don't need to review it now :)

@joethorley
Copy link
Contributor

@JessicaPenno - I think it needs to be added to the wqbc functions and then shinyrems will just need tweaking internally to make sure the data goes through this step.

@ateucher ateucher merged commit 3443294 into master Feb 19, 2021
@ateucher ateucher deleted the convert-mdl-units branch October 25, 2021 16:06
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.

3 participants