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

Update QC Functions #140

Merged
merged 7 commits into from
Aug 19, 2023
Merged

Update QC Functions #140

merged 7 commits into from
Aug 19, 2023

Conversation

jmcvey3
Copy link
Contributor

@jmcvey3 jmcvey3 commented Aug 4, 2023

Update CheckMonotonic to be more robust at finding all bad timestamps, and update RemoveFailedValues to be able to drop coordinate values.

Copy link
Collaborator

@maxwelllevin maxwelllevin left a comment

Choose a reason for hiding this comment

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

This looks pretty good! Could you update or add a test for the updated checker/handler?

@jmcvey3
Copy link
Contributor Author

jmcvey3 commented Aug 8, 2023

@maxwelllevin Done! Do you want to pin that version of pydantic to this PR as well?

@maxwelllevin
Copy link
Collaborator

@jmcvey3 yes, let's pin pydantic to <2.0.0 for now. I'll work on a second PR to update things to work with pydantic >=2 in the coming weeks

@jmcvey3
Copy link
Contributor Author

jmcvey3 commented Aug 16, 2023

@maxwelllevin It was pinned, but now we're looking at different errors. I can't replicate the ubuntu one on my machine, and can't tell for mac.

@codecov-commenter
Copy link

codecov-commenter commented Aug 18, 2023

Codecov Report

Merging #140 (6ac4ffc) into main (b7ecf81) will decrease coverage by 0.32%.
Report is 3 commits behind head on main.
The diff coverage is 66.66%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##             main     #140      +/-   ##
==========================================
- Coverage   89.16%   88.85%   -0.32%     
==========================================
  Files          28       28              
  Lines        2234     2261      +27     
  Branches      336      344       +8     
==========================================
+ Hits         1992     2009      +17     
- Misses        124      130       +6     
- Partials      118      122       +4     
Files Changed Coverage Δ
tsdat/qc/handlers.py 85.54% <40.00%> (-3.21%) ⬇️
tsdat/io/base.py 93.67% <63.63%> (-4.88%) ⬇️
tsdat/qc/checkers.py 83.64% <76.47%> (-0.50%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@maxwelllevin
Copy link
Collaborator

Well, that was partially successful. I think the issues are purely related to environment creation. I forgot that adi_py mac requires python >=3.10, so that's why that one is failing. I'm guessing that for the Ubuntu version a package wasn't installed correctly before, causing the NetCDF writer to crash. That seems to work now.

@maxwelllevin
Copy link
Collaborator

@jmcvey3 tests are passing now. I used the second workaround described in this issue to resolve it for mac, and it looks like that also helped the linux tests too: conda-incubator/setup-miniconda#274

@jmcvey3 jmcvey3 merged commit 2cc8add into main Aug 19, 2023
2 checks passed
@jmcvey3 jmcvey3 deleted the checkmonotonic branch August 19, 2023 01:13
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