Rework generated ragged array to utilize new conventions for coordinates #374
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Some tests. The following works but I get some surprising message: Same thing with: And this fails for me: |
selipot
requested changes
Feb 23, 2024
Merged
changeset: * add unit tests for gdp6h * fix bugs in gdp6h adapter code
|
@selipot warning about the missing fields and bug in the 6h adapter code has been fixed. I also added unit test for the 6 hourly dataset. On my previous commit I had overlooked the fact that the integration tests I had added weren't running with the CI command, this was because in my local environment I was running the newly added tests via |
|
@kevinsantana11 I tested again and it works however for each case the coordinate variable |
|
I am getting this error when I tried |
add in unit testing for gdp6h module
clouddrift/adapters/gdp6h.py
Outdated
| drifter_urls: list[str] = [] | ||
| for dir in directory_list: | ||
| urlpath = urllib.request.urlopen(os.path.join(url, dir)) | ||
| string = urlpath.read().decode("utf-8") | ||
| filelist = list(set(re.compile(pattern).findall(string))) | ||
| for f in filelist: | ||
| drifter_urls.append(os.path.join(url, dir, f)) | ||
| else: | ||
| drifter_urls = [f"{url}/{filename_pattern.format(id=did)}" for did in drifter_ids] |
There was a problem hiding this comment.
I don't think this can work, you are missing the /dir/ here. I think you might have removed a dictionnary we had before here that mapped id to a directory since those are not an direct match. So my fix (pasted below) is to get all the files looping the directories like you had it, and then keep only the selected drifter_ids if passed as an argument when creating the drifter_urls.
There was a problem hiding this comment.
I was actually going to push a modification that I just worked out, but since you change other stuff, I will just add it here. I replace this whole block with:
# create list of drifter urls
urlpath = urllib.request.urlopen(url)
string = urlpath.read().decode("utf-8")
drifter_urls: list[str] = []
for dir in directory_list:
urlpath = urllib.request.urlopen(os.path.join(url, dir))
string = urlpath.read().decode("utf-8")
filelist = list(set(re.compile(pattern).findall(string)))
for f in filelist:
if drifter_ids is None or int(f[:-3].split("_")[2]) in drifter_ids:
drifter_urls.append(os.path.join(url, dir, f))
changeset: update mypy config so new files are picked up fix bug on malformed url in windows
|
@kevinsantana11 not yet working for me. still returns string id variable and still returns |
|
Try reinstalling the branch, it is working for me. and |
|
Yes! It all seems to be working now! |
selipot
approved these changes
Mar 1, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #372 and reworks the coordinates to
id(traj)andtime(obs)