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

BUG: SSUSI bug and function deprecation #170

Merged
merged 13 commits into from
Jun 5, 2023
Merged

BUG: SSUSI bug and function deprecation #170

merged 13 commits into from
Jun 5, 2023

Conversation

aburrell
Copy link
Member

@aburrell aburrell commented May 19, 2023

Description

Fixes a bug identified when running several years of SSUSI data that revealed the dimensions sometimes change. Also deprecated a function introduced to pysat from this package, partially addressing issue #169.

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • Breaking change (fix or feature that would cause existing functionality
    to not work as expected)

How Has This Been Tested?

Added a unit test and also loaded a year of data (2010, one day at a time) for F16.

Test Configuration

Checklist:

  • Make sure you are merging into the develop (not main) branch
  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • Add a note to CHANGELOG.md, summarizing the changes
  • Update zenodo.json file for new code contributors

aburrell added 5 commits May 19, 2023 15:03
There are dimensions that are not always present in the SSUSI data, and so this kwarg must be False.
Added a deprecation warning to `expand_coords` and filtered in in the local calls so that users are only warned when they call the function directly.
Added a unit test for the new deprecation warning.
Updated the changelog.
Fixed issues revealed by CI flake8.
@aburrell
Copy link
Member Author

Failing due to issues addressed in #129, I believe.

@aburrell aburrell requested a review from jklenzing May 19, 2023 19:33
aburrell added 2 commits May 19, 2023 17:02
Added a catch for missing metadata when determining the fill value.
Added a missing variable to the full call.
@jklenzing
Copy link
Member

Since SSUSI data has not yet been released, do we need to deprecate the code? The next release here will be after pysat 3.1.0 and have that as a minimum req.

Copy link
Member

@jklenzing jklenzing left a comment

Choose a reason for hiding this comment

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

Verified that dmsp ssusi / timed guvi not present in last released version. Deprecation not needed since these functions have not yet been released, unless we are being super thorough.

Merging develop back into this should fix the setup issues.

@aburrell aburrell added this to the 0.0.5 Release milestone May 30, 2023
aburrell and others added 5 commits May 30, 2023 13:44
Develop update for SSUSI
Removed the local `expand_coords` function and replaced it with the new pysat `expand_xarray_dims` function.
Removed the deprecation tests, as they are no longer needed.
Removed deprecation warning from the changelog.
@aburrell
Copy link
Member Author

Waiting on pysat/pysat#1116 and pysat/pysat#1123 before tests will pass.

@jklenzing
Copy link
Member

Re-running with pysat 3.1.0, we get

 ValueError: coordinate 'Epoch' not present in all datasets.

for the timed saber data. This appears in the other pull too, so something probably changed with expand_dims. Or possibly there's another update I haven't seen yet

@jklenzing
Copy link
Member

jklenzing commented Jun 1, 2023

Found the bug, it was in issue with TIMED SABER revealed by the new exapnd_dims code. Out of scope here, working on a solution. Wrote up a summary in #174

@jklenzing
Copy link
Member

Merging in the recent changes from develop should fix the errors

@aburrell aburrell mentioned this pull request Jun 2, 2023
10 tasks
Develop update to SSUSI bug
@aburrell aburrell requested a review from jklenzing June 5, 2023 12:44
@aburrell aburrell merged commit 621f802 into develop Jun 5, 2023
@aburrell aburrell deleted the ssusi_bug_w_dep branch June 5, 2023 15:56
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.

2 participants