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

Issues/127: Updates to compute_coordinate_variable_names for grouped datasets #135

Merged
merged 11 commits into from
Dec 13, 2022

Conversation

frankinspace
Copy link
Member

@frankinspace frankinspace commented Dec 13, 2022

Github Issue: #127

Description

This change addresses the flattening of groups, to ensure subset and compute_coordinate_variable_names work on flattened groups in the same manner in the test suite.

Overview of work done

Encapsulated the opening of netcdf files (and group flattening) into a new function (subset.open_as_nc_dataset())and used that function throughout the test suite. The group flattening functions were also moved to a separate module for organization purposes.

Overview of verification done

Ran the test suites test_subset.py and test_run_subsetter.py locally, and checked which passed.

Overview of integration done

Explain how this change was integration tested. Provide screenshots or logs if appropriate. An example of this would be a local Harmony deployment.

PR checklist:

  • Linted
  • Updated unit tests
  • Updated changelog
  • Integration testing

See Pull Request Review Checklist for pointers on reviewing this pull request

frankinspace and others added 3 commits November 16, 2022 11:18
* move methods for flattening netcdf and hdf group structures to separate module

* feature/PODAAC-5065 (#129)

* fix way xarray open granules that have  as a time unit

* fix pylint

* change function to use original function if can parse only change units if we can not parse

* make xarray override into its own function

* add test for override_decode_cf_datetime function

* disable pyline one line instead of global

* Update podaac/subsetter/subset.py

Co-authored-by: Frank Greguska <[email protected]>

* add missing parameter to docstring

* typo in docstring

* extract netcdf opening procedure from beginning of `subset() into a new function

* update tests to use netcdf opening wrapper function, to prevent errors with tempo data

* /version 2.3.0-alpha.5

* update `test_specified_variables()` to use netcdf opening wrapper function in multiple places

to prevent errors with tempo data

* cosmetic

* clean up comment and use 'decode_times'=True for test

* feature/issue 126 (#131)

* Add variable leading slash flexibility

* Add tests back to test file

* changelog added and updated

* Update podaac/subsetter/subset.py

Co-authored-by: Frank Greguska <[email protected]>

* update Syntax

* resolve conflict

Co-authored-by: nlensse1 <[email protected]>
Co-authored-by: Frank Greguska <[email protected]>

* /version 2.3.0-alpha.6

* Update build-pipeline.yml

* /version 2.3.0-alpha.7

* Merge changes from origin/develop

* Merge changes from issues/127

Co-authored-by: sliu008 <[email protected]>
Co-authored-by: Frank Greguska <[email protected]>
Co-authored-by: l2ss-py bot <[email protected]>
Co-authored-by: Nick Lenssen <[email protected]>
Co-authored-by: nlensse1 <[email protected]>
@frankinspace frankinspace requested a review from a team December 13, 2022 02:32
Copy link
Member Author

I think I messed up the merge and lost some of the changes you made to the tests @danielfromearth. I think I can get them pulled back in here in a minute

Copy link
Collaborator

@jamesfwood jamesfwood left a comment

Choose a reason for hiding this comment

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

Looks good!

@danielfromearth
Copy link
Collaborator

Great, thanks @frankinspace for noticing that changes were missing and getting them back in there. I see you fixed some other errors with test_specified_variables too — awesome!
Thanks for reviewing too, @jamesfwood!

@frankinspace frankinspace merged commit e36f1e7 into develop Dec 13, 2022
@frankinspace frankinspace deleted the issues/127 branch December 13, 2022 20:03
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