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

update dem.py with new dem-stitcher api #339

Merged
merged 8 commits into from
Aug 5, 2022

Conversation

jlmaurer
Copy link
Collaborator

@jlmaurer jlmaurer commented May 2, 2022

dem-stitcher updated their API, which broke RAiDER. This PR fixes that issue. Some issues still remain but will be solved in another PR.

Description

  • changed function call to dem-stitcher

How Has This Been Tested?

  • all dem-related unit tests pass

Type of change

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

Checklist:

  • I have added an explanation of what your changes do and why you'd like us to include them.
  • I have written new tests for your core changes, as applicable.
  • I have successfully ran tests with your changes locally.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

@jlmaurer jlmaurer requested review from bbuzz31 and sssangha May 2, 2022 01:49
@jlmaurer
Copy link
Collaborator Author

jlmaurer commented May 2, 2022

Note that this probably won't pass all the tests, but it should pass everything except the scenario tests. If so @sssangha or @bbuzz31 it should be able to be merged.

@jlmaurer jlmaurer mentioned this pull request May 2, 2022
8 tasks
@jlmaurer
Copy link
Collaborator Author

jlmaurer commented May 2, 2022

Fixes #335

@jlmaurer jlmaurer mentioned this pull request May 2, 2022
@jlmaurer
Copy link
Collaborator Author

jlmaurer commented Jun 9, 2022

Closing this as the API was updated to the original (see release 2.1.0 from dem-stitcher). Will issue a new PR to update the funciton name.

@jlmaurer
Copy link
Collaborator Author

@sssangha this is looking good! Can you add the correct version dependency for dem-stitcher to the environment files? I'll try it on a clean install tomorrow then.

@jlmaurer
Copy link
Collaborator Author

@sssangha I'm thinking let's wait to merge this until the conda-forge package is available. We'll move the dependency to the conda install list at the same time then.

@sssangha
Copy link
Collaborator

@jlmaurer I went ahead and updated to update the latest release of dem-stitcher (i.e. >= 2.3)

@jlmaurer
Copy link
Collaborator Author

jlmaurer commented Aug 5, 2022

@sssangha this should be ready to merge! Can you approve it? Someone else is fine too. :) Thanks,

@jlmaurer jlmaurer requested review from cmarshak and dbekaert August 5, 2022 17:35
Copy link
Owner

@dbekaert dbekaert left a comment

Choose a reason for hiding this comment

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

LGTM

@jlmaurer jlmaurer merged commit a714654 into dbekaert:dev Aug 5, 2022
garlic-os added a commit to garlic-os/RAiDER that referenced this pull request Aug 15, 2022
autopep8
Housekeeping
Remove download_only path - Per Jeremy this logic is no longer used.
stash dep changes
update environment deps
correct name of dem_stitcher conda package
Merge pull request dbekaert#339 from jlmaurer/fix_stitchdem
update dem.py with new dem-stitcher api
Merge branch 'dev' into update_deps
remove uneeded deps
Update ERA5 API
Remove unneeded deps
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