Skip to content

update ERDDAP_IOOS_Sensor_Map#97

Merged
MathewBiddle merged 2 commits intoioos:mainfrom
ocefpaf:update_ERDDAP_IOOS_Sensor_Map
Nov 22, 2022
Merged

update ERDDAP_IOOS_Sensor_Map#97
MathewBiddle merged 2 commits intoioos:mainfrom
ocefpaf:update_ERDDAP_IOOS_Sensor_Map

Conversation

@ocefpaf
Copy link
Copy Markdown
Member

@ocefpaf ocefpaf commented Jul 11, 2022

Thank you for send a Pull Request to our code gallery! When adding or updating a notebook please check if:

  • The notebook has all the dependencies required to run in the IOOS env, if not please update the environment file.
  • You added a title, description, and a line with Created: YYYY-MM-DD in the first cell.
  • If you are updating a notebook add a line with Updated: YYYY-MM-DD below the created date.

Closes #91.

@review-notebook-app
Copy link
Copy Markdown

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@ocefpaf ocefpaf force-pushed the update_ERDDAP_IOOS_Sensor_Map branch from 33bbf48 to 8d4771b Compare July 11, 2022 19:42
@@ -8,34 +8,20 @@
"\n",
Copy link
Copy Markdown
Contributor

@MathewBiddle MathewBiddle Jul 12, 2022

Choose a reason for hiding this comment

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

Consider rephrasing. Something like: ... Let's investigate the datasets we found and try to figure out why there is a discrepancy.


Reply via ReviewNB

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done!

@@ -8,34 +8,20 @@
"\n",
Copy link
Copy Markdown
Contributor

@MathewBiddle MathewBiddle Jul 12, 2022

Choose a reason for hiding this comment

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

Spot on, but you can say it is because the sensor map only shows real-time observations (past 4-hours).

The Environmental Sensor Map integrates regional, national, and global real-time (past 4-hours) data across the IOOS enterprise.

Reply via ReviewNB

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I did not know that! I thought it was "recent" data but not ~4 hours.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think in practice it is "recent" (>4 hours). But the intention is it is all the realtime data with no guarantees for >4 hour data.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Got it. I made the changes and it is ready for another round of reviews.

@MathewBiddle
Copy link
Copy Markdown
Contributor

Other than my minor comments, this looks good to merge from my end. Thanks for refreshing this one!

@MathewBiddle
Copy link
Copy Markdown
Contributor

This slipped under the radar. I'm okay with merging and it doesn't look like this touches anything else.

@MathewBiddle MathewBiddle merged commit 47edd8d into ioos:main Nov 22, 2022
@ocefpaf ocefpaf deleted the update_ERDDAP_IOOS_Sensor_Map branch November 22, 2022 19:21
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.

Issue on page /content/code_gallery/data_access_notebooks/2017-03-21-ERDDAP_IOOS_Sensor_Map.html

2 participants