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.

Sign up for GitHub

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

Jump to bottom

Fix OSError on _nc_dataset method #271

Closed
wants to merge 10 commits into from
Closed

Fix OSError on _nc_dataset method #271

wants to merge 10 commits into from

Conversation

vinisalazar
Copy link
Contributor

Hi,

this is in relation to #261.

Summary of changes

Thanks,
V

@vinisalazar
Fix OSError on _nc_dataset method
061bf1a 
  - Call data.seek(0) after reading 'data' object for the first time (Closes ioos#261)
  - Update 'test_to_objects' to use gold standard Sensors server from ioos#258
  - Rewrite cassette
@vinisalazar
Add delete=False flag in _tempnc
0ea49e2 
  - Fix from bravoserver/bravo#111
  - This error is further documented on Stack Overflow (see ioos#261 for discussion)
@vinisalazar
Fix _tempnc function on Windows
a798209 
  - Set delete=True on tmp instead of manually unlinking
ocefpaf
ocefpaf reviewed Oct 4, 2022
View reviewed changes
tests/test_to_objects.py
import httpx
import iris
import pytest
import xarray as xr

from erddapy import ERDDAP

dask.config.set(scheduler="single-threaded")
Copy link
Member

Choose a reason for hiding this comment

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

This is a better workaround for the netcdf 1.6.1 problem. Netcdf-c is not thread safe and we should not rely on it.

ocefpaf
ocefpaf reviewed Oct 4, 2022
View reviewed changes
erddapy/core/netcdf.py

# Delete flag must be False when using Windows
# The file will be closed in the 'finally' block
delete = False if platform == "win32" else True
Copy link
Member

Choose a reason for hiding this comment

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

Looks like this is not working. Was this failing on Windows before?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The CI started failing when I introduced the data.seek(0) line in 061bf1a

vinisalazar added a commit to vinisalazar/erddapy that referenced this pull request Oct 10, 2022
@vinisalazar
Another attempt at fixing tests
07f392c 
  - Switching tests to gold standard server (ioos#261)
  - Will rebase ioos#271 after merging
vinisalazar added a commit to vinisalazar/erddapy that referenced this pull request Oct 10, 2022
@vinisalazar
Another attempt at fixing tests
72df243 
  - Switching tests to gold standard server (ioos#261)
  - Will rebase ioos#271 after merging
This was referenced Oct 10, 2022
Merged
Draft
ocefpaf pushed a commit to vinisalazar/erddapy that referenced this pull request Oct 14, 2022
@vinisalazar @ocefpaf
Another attempt at fixing tests
e8722e2 
  - Switching tests to gold standard server (ioos#261)
  - Will rebase ioos#271 after merging
@ocefpaf
Copy link
Member

ocefpaf commented Oct 14, 2022

@vinisalazar in light of #272 do we still need this one?

@vinisalazar
Copy link
Contributor Author

I don't think we do. Thanks!

@ocefpaf ocefpaf closed this Oct 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ocefpaf ocefpaf ocefpaf left review comments

Assignees
No one assigned
Labels
GSoC22 hacktoberfest-accepted
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

to_xarray method error on OS X
2 participants
@vinisalazar @ocefpaf