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

Change functionality of read_array so it returns multiple variables #78

Merged
merged 2 commits into from
Jan 29, 2024

Conversation

Suizer98
Copy link
Contributor

To address #77

After modifying this will enable us to perform:

variables = ["longitude", "latitude", "time"]
result = nco.ncks(
    input="sample.nc", options=["-v " + ",".join(variables)], returnArray=variables
)

lon = result["longitude"][:]
lat = result["latitude"][:]
...

so we don't have to call returnArray=[] one by one.

@czender czender self-assigned this Jan 28, 2024
@czender
Copy link
Member

czender commented Jan 28, 2024

@Suizer98 Thank you for your patch. As you can see, the same error occurs in all the above test results. It looks like a test fails to find a variable that presumably it used to find. Please update your branch/PR and ping me when it's ready to retest. There is also a deprecation warning that I do not recall seeing before. It likely has nothing to do with your branch. Feel free to fix that at the same time if you want, otherwise I'll give it a try.

@Suizer98
Copy link
Contributor Author

@Suizer98 Thank you for your patch. As you can see, the same error occurs in all the above test results. It looks like a test fails to find a variable that presumably it used to find. Please update your branch/PR and ping me when it's ready to retest. There is also a deprecation warning that I do not recall seeing before. It likely has nothing to do with your branch. Feel free to fix that at the same time if you want, otherwise I'll give it a try.

Looks like the new changes only recognize list of strings instead of single string, I will try to add a new condition in a new commit.

@czender
Copy link
Member

czender commented Jan 28, 2024

@Suizer98 Looks like good progress before a new issues arises...

@Suizer98 Suizer98 force-pushed the master branch 2 times, most recently from ae6a1b5 to 68a0f96 Compare January 29, 2024 02:15
@Suizer98
Copy link
Contributor Author

I amended the commit accordingly and ran pytest to ensure that the changes didn't introduce any new issues. Everything should appear to be functioning as expected now. Feel free to check.

I made sure the classic way read_array works as usual:

result = nco.ncks(
    input="fake-trim.nc", returnArray="longitude"
)

@czender czender merged commit 9bed5b5 into nco:master Jan 29, 2024
6 checks passed
@czender
Copy link
Member

czender commented Jan 29, 2024

You did it. Touchdown! Many thanks for your contribution.

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.

None yet

2 participants