dOTC

[LamAdr]

Pull Request Checklist:

• This PR addresses an already opened issue (for bug fixes / features)
  • This PR fixes #xyz
• Tests for the changes have been added (for bug fixes / features)
  • (If applicable) Documentation has been added / updated (for bug fixes / features)
• CHANGES.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?

• Introduce dOTC bias correction method into sdba.

Does this PR introduce a breaking change?

No

Other information:

Am I doing this right?

[github-actions]

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 ✨

[Zeitsperre]

Am I doing this right?

You are!

Once you address the errors brought up by pre-commit, the tests should run. From what I can tell, there are a few programming errors and some style problems. To make things easier, try installing pre-commit beforehand to have the checks run when committing:

$ pip (or conda) install pre-commit
$ pre-commit install

[LamAdr]

NB : I added "ot" to codespell's ignore list

[coxipi]

Very nice! The code is very readable, well organized, good job!

In one of my comment where I mention a possible function _otc_adjust, you can also see I mention map_blocks. Basically, your are currently doing the correction (for OTC):

` tasmax_1950-1980, pr_1950-1980 -> [pr_1950-1980]', [pr_1950-1980]' ` 

where the full 30 years of climatology are used in training/adjusting.

It is often useful to have different time grouping, e.g. put all January 1rst together and correct them apart, put all Jan. 2nd together, correct them separately, etc.

` tasmax_01-01-19XY, pr_01-01-19XY -> [pr_01-01-19XY]', [pr_01-01-19XY]' ` 
` tasmax_02-01-19XY, pr_02-01-19XY -> [pr_02-01-19XY]', [pr_02-01-19XY]' ` 

If you look at class EmpiricalQuantileMapping, you will see that eqm_train can receive the argument "group". To reproduce the case you were doing, you set:

group = "time"

to have the second example I'm describing, you would set:

group = "time.dayofyear"

this tells to the code, group data in days of year, i.e. Jan.1, Jan.2, Jan.3, etc. This would be nice to have.

[SarahG-579462]

Looks good!

[LamAdr]

@Zeitsperre
I guess the macos fails because SBCK isn't installed.
Do you understand why Python3.9 (ubuntu-latest, conda) fails?

[Zeitsperre]

@LamAdr No worries, there's some behaviour change in the latest version of dask and it's impacting both conda and macOS builds right now. The fix is already in #1870 and will be merged soon.

[Zeitsperre]

@Zeitsperre I guess the macos fails because SBCK isn't installed.

Indeed. You need to add another pytest.importorskip on the affected tests. SBCK is only installed in two builds on CI.

[Zeitsperre]

@LamAdr Is this pull request ready? If so, it'll be the last one before we tag a new release (ideally, tomorrow).

Let me know if you want more time to do any finishing touches!

[LamAdr]

@Zeitsperre Yep, ready.
Thanks again