-
Notifications
You must be signed in to change notification settings - Fork 27
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
Fix tests to passing #173
Fix tests to passing #173
Conversation
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.
@cassandra-elmer do you mean getting the files to pass the IOOS compliance checker?
I could have sworn the polar changes were already made, but I must not have pushed them...
I'll ope an issue to add the compliance checker to the tests. I think that can be done routinely so we don't get out of sync with them.
tests/test_seaexplorer.py
Outdated
kind='sub') | ||
assert 'No such file or directory' in str(missing_file_exc) | ||
assert result_sub == 'tests/data/l0-profiles/dfo-eva035-20190718.nc' | ||
|
||
|
||
def test_missing_bad_timebase(): | ||
# Prepare yaml files with bad timebase and no timebase | ||
with open(example_dir / 'example-seaexplorer/deploymentRealtime.yml') as fin: | ||
with open(str(example_dir / 'example-seaexplorer/deploymentRealtime.yml')) as fin: |
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 you explain why this is needed? open
should work with Path variables.
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.
@cassandra-elmer do you mean getting the files to pass the IOOS compliance checker?
The tests I meant are these
Working on the compliance checker is my own next step, but I wanted to fix the above tests first.
Can you explain why this is needed?
open
should work with Path variables.
Those are likely redundant because I got trigger happy, yes. There were previous example_dir / '*.yml
spots that weren't open
and were breaking and I just carried it through.
@@ -58,12 +58,12 @@ def test_merge_rawnc(): | |||
result_default = seaexplorer.merge_parquet( | |||
'tests/data/realtime_rawnc/', | |||
'tests/data/realtime_rawnc/', | |||
example_dir / 'example-seaexplorer/deploymentRealtime.yml') | |||
str(example_dir / 'example-seaexplorer/deploymentRealtime.yml')) |
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 file still has the str
conversions as well.... We shouldn't need these. Thanks!
Thanks @cassandra-elmer ! |
Edits to processing scripts, example NetCDFs, and deployment YAMLs to make tests pass.
The majority of changes are adjusting variable attributes from strings to floats to match existing pyglider code (and the IOOS GliderDAC requirements). Other changes include ensuring variables of type Path are input to functions as type str and updating keyword arguments to reflect expected inputs according to current function documentation.