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

Updates to improve compliance with the IOOS Compliance Checker (glide… #163

Closed
wants to merge 104 commits into from

Conversation

jcermauwedu
Copy link

…rdac:3.0)

  • Still need to ignore ancillary_variable recommendations or skip specific test
  • ncprocess.py
    • profile_id: Max int32 type; attributes valid max and min also need to be same type; _FillValue = -999
    • platform: Max int32 type; _FillValue = -999
    • set time encoding to float to avoid UserWarnings about time conversion
  • slocum.py
    • If import test fails for dbdreader, the flag should be False for have_dbdreader?
    • Need pandas to help avoid UserWarnings about time conversion; set encoding to float for time encoding
    • depth: valid min/max and _FillValue updates to match gliderdac:3.0
    • Other updates from char to float to match gliderdac:3.0
    • Obtain direct version of libnetcdf (borrow code from xarray)
  • tests
    • Add README to discover location of example data

Other things that should be added/updated to respective deployment.yml files to improve compliance. Once that is done, things are squeaky clean.

$ compliance-checker -t gliderdac:3.0 -s check_ancillary_variables pyglider/L0-profiles/unit507-20240325T1507.nc
Running Compliance Checker on the datasets from: ['pyglider/L0-profiles/unit507-20240325T1507.nc']

--------------------------------------------------------------------------------
                         IOOS Compliance Checker Report
                                Version unknown
                     Report generated 2024-03-26T00:50:22Z
                                 gliderdac:3.0
      https://ioos.github.io/glider-dac/ngdac-netcdf-file-format-version-2
--------------------------------------------------------------------------------
All tests passed!

…rdac:3.0)

 * Still need to ignore `ancillary_variable` recommendations
 * ncprocess.py
   * profile_id: Max int32 type; attributes valid max and min also need to be same type; _FillValue = -999
   * platform: Max int32 type; _FillValue = -999
   * set time encoding to float to avoid UserWarnings about time conversion
 * slocum.py
   * If import test fails for dbdreader, the flag should be False for have_dbdreader?
   * Need pandas to help avoid UserWarnings about time conversion; set encoding to float for time encoding
   * depth: valid min/max and _FillValue updates to match gliderdac:3.0
   * Other updates from char to float to match gliderdac:3.0
   * Obtain direct version of libnetcdf (borrow code from xarray)
 * tests
   * Add README to discover location of example data

Other things that should be added/updated to respective deployment.yml files to improve
compliance.  Once that is done, things are squeaky clean.

```
$ compliance-checker -t gliderdac:3.0 -s check_ancillary_variables pyglider/L0-profiles/unit507-20240325T1507.nc
Running Compliance Checker on the datasets from: ['pyglider/L0-profiles/unit507-20240325T1507.nc']

--------------------------------------------------------------------------------
                         IOOS Compliance Checker Report
                                Version unknown
                     Report generated 2024-03-26T00:50:22Z
                                 gliderdac:3.0
      https://ioos.github.io/glider-dac/ngdac-netcdf-file-format-version-2
--------------------------------------------------------------------------------
All tests passed!
```
@jklymak jklymak force-pushed the main branch 3 times, most recently from fc6deb2 to 9cec8bf Compare March 28, 2024 01:34
jklymak and others added 25 commits September 22, 2024 09:35
 * We do not want to ignore the first line of any data
 * Do not interpolate the data (get_sync -> get)
 * .gitignore: Ignore build directory
 * Ignore build directory
 * Add gitignore files and directories created by running pytest
 * Allow setting time and calendar units from yml configuration file
 * Fix to allow profile_time_start and profile_time_end to get time
   information by moving time_1970 to last variable processed in
   for loop
 * Emit warning for variables lacking data for gridding
 * Add resync_metadata_and_encoding to allow overriding of metadata
   from yml configuration file
 * Fix log message for raw_to_timeseries
 * Allow conversion of slocum latitude and longitude from NMEA to degrees
 * Emit log message when metadata is overridden from yml configuration file
 * Add utils function to determine pyglider version
 * Add utils function to determine netcdf C and python versions
 * Set units for profile_time in example-slocum/deploymentRealtime.yml to
   get proper units for profile_time
 * Remove history and netcdf attributes as they can now vary widely from
   static examples
@jr3cermak jr3cermak mentioned this pull request Dec 6, 2024
@VeckoTheGecko
Copy link
Contributor

VeckoTheGecko commented Dec 17, 2024

6651931 (via #210) introduced a bunch of formatting changes that will likely cause merge conflicts. Here are merge and rebase workflows to avoid these conflicts (it depends on having pre-commit installed - see the contributing docs).

I recommend first creating a temp branch off of your branch of interest to test these workflows and make sure the output is as expected. I have tested these and they seem to work well.

# merge all the way up to the commit that introduced the formatting changes
git merge 66519318^1

# merge the commit that introduced the formatting changes, prioritising the working branch during conflicts
git merge -s ours 66519318 --no-commit 
pre-commit run --all-files
git commit --no-edit

# merge the rest of the way
git merge main

or by doing a rebase (if you want a clean history)

# Rebase up to the commit introducing formatting changes
git rebase 66519318^1

# Rebase onto the commit introducing formatting changes.
# Accept working branch changes (i.e., "theirs" during rebase
# - different to merge. See git rebase docs) during conflicts.
# During the rebase we will run the formatter after applying
# changes from commits, before committing so that everything is compliant.
git rebase -X theirs -i 66519318
# Replace all "pick" with "edit" or "e". Start the rebase...

# Repeatedly run
git add -u && pre-commit run --all-files && git rebase --continue
# to run pre-commit tooling, stage modified files, and continue
# until rebase is complete. Sometimes pre-commit can't autofix
# issues, then you need to fix them manually.
# If you make a mistake you can always do `git rebase --abort`

# Finally, rebase onto main
git rebase main

@jklymak
Copy link
Member

jklymak commented Dec 17, 2024

I'm going to close this as outdated. I'm pretty sure the current pyglider is compatible with ioos gdac. If there are remaining incompatibilities, let's address with a new PR.

@jklymak jklymak closed this Dec 17, 2024
@jr3cermak
Copy link

6651931 (via #210) introduced a bunch of formatting changes that will likely cause merge conflicts. Here are merge and rebase workflows to avoid these conflicts (it depends on having pre-commit installed - see the contributing docs).

I recommend first creating a temp branch off of your branch of interest to test these workflows and make sure the output is as expected. I have tested these and they seem to work well.

# merge all the way up to the commit that introduced the formatting changes
git merge 66519318^1

# merge the commit that introduced the formatting changes, prioritising the working branch during conflicts
git merge -s ours 66519318 --no-commit 
pre-commit run --all-files
git commit --no-edit

# merge the rest of the way
git merge main

or by doing a rebase (if you want a clean history)

# Rebase up to the commit introducing formatting changes
git rebase 66519318^1

# Rebase onto the commit introducing formatting changes.
# Accept working branch changes (i.e., "theirs" during rebase
# - different to merge. See git rebase docs) during conflicts.
# During the rebase we will run the formatter after applying
# changes from commits, before committing so that everything is compliant.
git rebase -X theirs -i 66519318
# Replace all "pick" with "edit" or "e". Start the rebase...

# Repeatedly run
git add -u && pre-commit run --all-files && git rebase --continue
# to run pre-commit tooling, stage modified files, and continue
# until rebase is complete. Sometimes pre-commit can't autofix
# issues, then you need to fix them manually.
# If you make a mistake you can always do `git rebase --abort`

# Finally, rebase onto main
git rebase main

@VeckoTheGecko Thank you for looking at this. As mentioned in the OceanGlider DMTT, this PR will be rebuilt starting from the current master branch and resubmitted later. At the time of this original PR, the test harness and sample data were being pushed around. It will be easier to start anew.

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.

6 participants