-
-
Notifications
You must be signed in to change notification settings - Fork 402
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
Update cdms table #2760
Update cdms table #2760
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2760 +/- ##
==========================================
- Coverage 66.11% 66.10% -0.02%
==========================================
Files 234 234
Lines 18048 18057 +9
==========================================
+ Hits 11933 11937 +4
- Misses 6115 6120 +5
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
@bsipocz this is ready for review. lmk if you think more tests are needed |
The docs build issue is a genuine one. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs a changelog, and the docs build and tests fixed.
Maybe some test coverage would be nice, too as the new functionality is not tested anywhere. If you think its unnecessarily big download, then we can package up a much smaller test file in data and use its github url.
@@ -1,1089 +1,1198 @@ | |||
TAG | NAME | NLINE | lg(Q(1000)) | lg(Q(500)) | lg(Q(300)) | lg(Q(225)) | lg(Q(150)) | lg(Q(75)) | lg(Q(37.5)) | lg(Q(18.75)) | lg(Q(9.375)) | lg(Q(5.000)) | lg(Q(2.725)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There has been a lot of back and forth with this file, please squash those changes out into one commit touching the datafile.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we just squash at the end?
Only remaining red is code coverage. |
Ok, could you add a changelog, and squash the datafile modifications? |
Thanks @keflavich! |
The CDMS catalog directory table has been updated. This PR updates our cached file & implements a tool for future updating & makes it possible to use the live, remote version. The only reason this didn't exist previously was my own laziness; the formatting was weird b/c of html stuff mixed in with the table and I had just hand hacked it to fit. Turns out it's pretty easy to read the table with
ascii.read
, just took some figuring.