Closed ocefpaf closed 4 months ago
@MathewBiddle this one is ready for review. I don't want to add more metrics to avoid making it to hard for reviewing. I'll add 3-4 metrics per PR to keep the context short. Is that OK?
I like the direction this is going. Thanks @ocefpaf. I'd like to test this locally before I say anything else, however.
@ocefpaf this is looking good. Did you want to add any more to this PR, or should we merge and move to the next batch?
@ocefpaf this is looking good. Did you want to add any more to this PR, or should we merge and move to the next batch?
Let's merge and move to the next batch. I have a few extra commits here that could use a rebase and a new PR to avoid confusion.
@MathewBiddle these are the first functions I extracted from IOOS_BTN.ipynb. I'll do the rest soon but I'd like to get your input before moving forward with this idea. For now you can run it with:
When we have all the metrics as their own function we can:
This will also be easier to maintain and review as individual pieces. Here, for example, I removed on dependency (
bs4
) , two extra steps, and fixed a pandas deprecation when creating the table to make it future proof.