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

perform flattening as necessary in test suite #130

Merged
merged 18 commits into from
Dec 13, 2022

Conversation

danielfromearth
Copy link
Collaborator

@danielfromearth danielfromearth commented Nov 28, 2022

Github Issue: #127

Description

Summarize the ticket here

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

Summarize the work you did

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

Summarize the testing and verification you've done. This includes unit tests or testing with specific data

Ran the test suites test_subset.py and test_run_subsetter.py locally, and checked which passed. Note that the test_get_time_variable_name() function is failing for TEMPO NO2 example data, but it seems to be because of the time units rather than an issue with the group structure (because it fails on a call to xr.open_dataset() when decoding times).

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

@danielfromearth danielfromearth changed the title move methods for flattening netcdf and hdf group structures to separa… make group flattening consistent Nov 28, 2022
sliu008 and others added 6 commits December 8, 2022 10:13
* 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]>
@danielfromearth danielfromearth changed the base branch from develop to issues/127 December 8, 2022 18:42
…ction in multiple places

to prevent errors with tempo data
@danielfromearth danielfromearth changed the title make group flattening consistent perform flattening as necessary in test suite Dec 8, 2022
nlenssen2013 and others added 8 commits December 12, 2022 09:24
* 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]>
# Conflicts:
#	podaac/subsetter/subset.py
# Conflicts:
#	podaac/subsetter/subset.py
#	tests/test_subset.py
@frankinspace
Copy link
Member

@danielfromearth thank you for the contribution! I think your approach makes sense. Just a couple housekeeping items:

It looks like you targeted the issues/127 branch as the destination of your PR. This was probably because you didn't have push access to the branch directly? Preferably you could have contributed directly to that branch so I'll reach out to get you that access. Normally we would have feature/issues branches target develop so that the CI/CD pipeline can run on them.

I've merged the two branches together now so I'll accept it into issues/127 and then I think we'll be able to go to develop from there. Thanks again!

@frankinspace frankinspace merged commit f8122d4 into podaac:issues/127 Dec 13, 2022
@danielfromearth danielfromearth deleted the Feature/issue_127 branch December 13, 2022 15:17
frankinspace added a commit that referenced this pull request Dec 13, 2022
…datasets (#135)

* Update tests to use parameterized pytest. Some tests failing on new TEMPO test dataset

* Replace / in all var names if the dataset is grouped

* perform flattening as necessary in test suite (#130)

* 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]>

* Run verification on issue branches

* Updated changelog

* add module docstring

* fix import statements

* fix import statements

* Fix tests

Co-authored-by: Daniel Kaufman <[email protected]>
Co-authored-by: sliu008 <[email protected]>
Co-authored-by: l2ss-py bot <[email protected]>
Co-authored-by: Nick Lenssen <[email protected]>
Co-authored-by: nlensse1 <[email protected]>
Co-authored-by: danielfromearth <[email protected]>
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.

4 participants