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

Split from xclim #8

Open
wants to merge 125 commits into
base: main
Choose a base branch
from
Open

Split from xclim #8

wants to merge 125 commits into from

Conversation

coxipi
Copy link

@coxipi coxipi commented Jul 25, 2024

Pull Request Checklist:

  • This PR addresses an already opened issue (for bug fixes / features)
    • This PR fixes #xyz
  • (If applicable) Documentation has been added / updated (for bug fixes / features).
  • (If applicable) Tests have been added.
  • CHANGELOG.rst has been updated (with summary of main changes).
    • Link to issue (:issue:number) and pull request (:pull:number) has been added.

What kind of change does this PR introduce?

  • First pass on splitting sdba from xclim. Mostly all tests that previously existed are passed in this separate module. Functions and tests that we very climate specific were removed.
  • calendar and units functions of xclim are ported here. Since the idea was to have xclim as an optional depedency, there is some redundancy
  • Some xclim utils necessary for e.g. properties are only available if xclim is installed. The core modules such as adjustment.py
  • Testing suite requires xclim
  • Notebooks require xclim (mainly to import testing data, this could change in the future)

Does this PR introduce a breaking change?

N/A

Other information:

Important parts that still need attention:

  • units.py copies a lot of stuff and the end of base.py. Is that all right?

Copy link

github-actions bot commented Jul 25, 2024

Welcome, new contributor!

It appears that this is your first Pull Request. To give credit where it's due, we ask that you add your information to the AUTHORS.rst and .zenodo.json:

  • The relevant author information has been added to AUTHORS.rst and .zenodo.json.

Please make sure you've read our contributing guide. We look forward to reviewing your Pull Request shortly ✨

@github-actions github-actions bot added the docs label Jul 25, 2024
@github-actions github-actions bot added the CI label Jul 25, 2024
.readthedocs.yml Outdated Show resolved Hide resolved
@Zeitsperre
Copy link
Contributor

@coxipi I believe the new failures are due to the latest xarray.

tox.ini Outdated Show resolved Hide resolved
@@ -61,6 +61,6 @@ commands_pre =
pip list
pip check
commands =
pytest --cov
pytest xsdba {posargs}
Copy link
Contributor

Choose a reason for hiding this comment

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

This is failing because there is no xsdba folder in the top-level (it lives in src). Also, pytest is looking for tests.

The --cov argument takes a keyword (xsdba) to know that the relevant module ton look at is the library. --cov on its own is an error.

Suggested change
pytest xsdba {posargs}
pytest {posargs}

@@ -42,9 +40,11 @@ commands =
allowlist_externals =
make

[pytest]
addopts = --color=yes --cov
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
addopts = --color=yes --cov
addopts = --color=yes --cov=xsdba

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants