search

MESMER-group / mesmer

spatially-resolved ESM-specific multi-scenario initial-condition ensemble emulator
https://mesmer-emulator.readthedocs.io/en/latest/
GNU General Public License v3.0
22 stars 15 forks source link

Remove power transformer class #474

Closed veni-vidi-vici-dormivi closed 3 days ago

veni-vidi-vici-dormivi commented 1 week ago

In this PR I am removing the power transformer class in favor of the newly written xarray functions and adjusting the tests accordingly.

veni-vidi-vici-dormivi commented 1 week ago

I now deleted the PowerTransformerVariableLambda class. Should we instead deprecate it properly? I thought it is unnecessary because because it was never properly implemented. What do you think @mathause?

codecov[bot] commented 1 week ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 51.53%. Comparing base (9b0b76b) to head (4745f11). Report is 64 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #474 +/- ## =========================================== - Coverage 87.90% 51.53% -36.38% =========================================== Files 40 50 +10 Lines 1745 3252 +1507 =========================================== + Hits 1534 1676 +142 - Misses 211 1576 +1365 ``` | [Flag](https://app.codecov.io/gh/MESMER-group/mesmer/pull/474/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MESMER-group) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/MESMER-group/mesmer/pull/474/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MESMER-group) | `51.53% <ø> (-36.38%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MESMER-group#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

veni-vidi-vici-dormivi commented 3 days ago

I think the only point where we still need sklearn now is the tests. So do I see that correctly that we could now (again) have a separate environment for testing and for the users and exclude sklearn in the users environment, or not? Even if so, I would keep it at one environment at this point I think.

mathause commented 3 days ago

It's always nice to have less dependencies but we can keep it for the moment. Maybe add a comment in setup.cfg?