Skip to content

starting work on yearly RA GTS metrics #120

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

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

MathewBiddle
Copy link
Contributor

Starting to generate yearly RA metrics

will close #118

@MathewBiddle MathewBiddle marked this pull request as ready for review May 14, 2025 13:08
@MathewBiddle MathewBiddle requested a review from ocefpaf May 14, 2025 13:08
@MathewBiddle
Copy link
Contributor Author

@ocefpaf this notebook is simply working on some calculations based on our NDBC GTS metrics. I don't know if this will be something to add to the website yet. But, I'm sure there are ways we can make this more understandable.

As it stands, it looks more like a note document instead of a notebook someone could potentially reuse. So, I'm open to suggestions for improvements.

@ocefpaf
Copy link
Member

ocefpaf commented May 15, 2025

As it stands, it looks more like a note document instead of a notebook someone could potentially reuse. So, I'm open to suggestions for improvements.

Looks good. The notebooks could use some reduction [1] and style unification [2], it is all over the place... I'm guessing AI generated code? The main concern is with the 2 spaces mixed with 4, that can be dangerous if changing the code in those loops and not paying attention. One can get wring results if a line ends up in the wrong place, best case scenario, it will crash.

I made a few changes, but I won't send to this PR unless you are OK with it.

Are the functions get_ioos_regional_data and get_ioos_regional_yearly_stats something you plan to use in the notebook? At the moment they are not, right?

  1. By reduction I mean remove the, what I believe is some data exploration, and create something that return either the final metric or a dataframe used by multiple other reporting tools. As it stands, the notebook does too much and it is hard to follow.
  2. We can activate some notebook linting for the style here if you want. Some people don't like it, but I believe it helps with maintainability of the code by increasing the notebook readability.

@ocefpaf
Copy link
Member

ocefpaf commented May 15, 2025

BTW, we have a pixi setup for this repo. If you have pixi installed locally you can do:

pixi add erddapy tabulate

Then pixi shell to "activate" the environment and call the jupyter notebook as you normally do.

@MathewBiddle
Copy link
Contributor Author

I should also provide some context. I generated this notebook out of an abundance of questions about the metrics we have on data going to the GTS. So, when a new question came in, I created a new cell and ran with it. So, yes it is all over the place and I can't even track what's happening.

The last two cells were AI generated, just to see what it could do and if the results match my expectation. More for testing.

As for the two functions in create_gts_regional_landing_page.py, those can be removed from this PR. I started going down the road of figuring out how we might add these additional data to the webpage, so I built functions to gather them as a holding space. I don't think we're ready to put something on the website just yet, so let's drop it. I'll send that change.

@ocefpaf
Copy link
Member

ocefpaf commented May 15, 2025

I generated this notebook out of an abundance of questions about the metrics we have on data going to the GTS. So, when a new question came in, I created a new cell and ran with it. So, yes it is all over the place and I can't even track what's happening.

Normal developing cycle. The problem is human Matt hours to follow up on that later 😄

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.

Total # observations per year submitted by IOOS RAs to NDBC
2 participants