linslope #307

raybellwaves · 2021-05-07T21:13:26Z

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Closes #284 (linslope)

Type of change

Please delete options that are not relevant.

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)
Performance (if you touched existing code run asv to detect performance changes)
refactoring

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. This could point to a cell in the updated notebooks. Or a snippet of code with accompanying figures here.

Checklist (while developing)

I have added docstrings to all new functions.
I have commented my code, particularly in hard-to-understand areas
Tests added for pytest, if necessary.

Pre-Merge Checklist (final steps)

I have rebased onto main or develop (wherever I am merging) and dealt with any conflicts.
I have squashed commits to a reasonable amount, and force-pushed the squashed commits.

References

Please add any references to manuscripts, textbooks, etc.

review-notebook-app · 2021-05-07T21:13:30Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

pep8speaks · 2021-05-07T21:13:31Z

Hello @raybellwaves! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-05-08 23:44:06 UTC

raybellwaves · 2021-05-07T22:05:55Z

failures aren't related this PR AFAICT

https://github.com/xarray-contrib/xskillscore/runs/2531371780?check_suite_focus=true#step:7:11563

https://github.com/xarray-contrib/xskillscore/blob/main/xskillscore/tests/test_probabilistic.py#L872

see #308

aaronspring · 2021-05-07T22:11:25Z

do we really need this? isnt http://xarray.pydata.org/en/latest/generated/xarray.DataArray.polyfit.html#xarray.DataArray.polyfit enough?
see https://github.com/bradyrx/esmtools/blob/20c005d49fc1875288cf68c24bf61bd9487c0531/esmtools/stats.py#L335

aaronspring

implementation looks good to me

raybellwaves · 2021-05-07T22:27:24Z

Thanks i'll checkout polyfit. Could you give me a drop in of it for the following:

import xarray as xr
import pandas as pd
import numpy as np
import xskillscore as xs
from scipy.stats import linregress

stores = np.arange(100)
skus = np.arange(100)
dates = pd.date_range("1/1/2020", "1/10/2020", freq="D")

rows = []
for _, date in enumerate(dates):
    for _, store in enumerate(stores):
        for _, sku in enumerate(skus):
            rows.append(
                dict(
                    {
                        "DATE": date,
                        "STORE": store,
                        "SKU": sku,
                        "QUANTITY_SOLD": np.random.randint(9) + 1,
                    }
                )
            )
df = pd.DataFrame(rows)
df.rename(columns={"QUANTITY_SOLD": "y"}, inplace=True)
df.set_index(['DATE', 'STORE', 'SKU'], inplace=True)
noise = np.random.uniform(-1, 1, size=len(df['y']))
df['yhat'] = (df['y'] + (df['y'] * noise)).astype(int)

df.groupby(['STORE', 'SKU']).apply(lambda x: linregress(x.y, x.yhat)[0])

ds = df.to_xarray()
ds.xs.linslope('y', 'yhat', 'DATE')

Am aware of @bradyrx's implementation of this which grabs all output. Similar to pearson_r and pearson_r_p_value I tend favor separate functions.

My motivation here is the example above (taken from raybellwaves/xskillscore-tutorial#3). I have a pandas.DataFrame and I want to do some metrics on it including slope.

aaronspring · 2021-05-07T22:31:18Z

What do you mean by drop in on it? Feedback?

raybellwaves · 2021-05-07T23:48:11Z

How to use polyfit as a replacement for linregress (linslope) on xarray objects. I see there isn’t an example in the docstring

…

On Fri, May 7, 2021 at 6:31 PM Aaron Spring ***@***.***> wrote: What do you mean by drop in on it? Feedback? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#307 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEC6DZGLOLJ3ZGSZOHZS5ATTMRS4RANCNFSM44LBZ7AA> .

aaronspring · 2021-05-08T06:31:20Z

How to use polyfit as a replacement for linregress (linslope) on xarray
objects. I see there isn’t an example in the docstring

On Fri, May 7, 2021 at 6:31 PM Aaron Spring @.***>
wrote:

What do you mean by drop in on it? Feedback?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#307 (comment),
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AEC6DZGLOLJ3ZGSZOHZS5ATTMRS4RANCNFSM44LBZ7AA
.

pydata/xarray#5278

But I think less functionality than linregress, so go on with this PR

raybellwaves · 2021-05-08T10:24:15Z

failures addressed in #309

codecov · 2021-05-08T20:38:20Z

Codecov Report

Merging #307 (6a76bdc) into main (54ff8f2) will increase coverage by 0.07%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #307      +/-   ##
==========================================
+ Coverage   94.50%   94.57%   +0.07%     
==========================================
  Files          23       23              
  Lines        2566     2601      +35     
==========================================
+ Hits         2425     2460      +35     
  Misses        141      141

Impacted Files	Coverage Δ
xskillscore/__init__.py	`84.61% <ø> (ø)`
xskillscore/tests/test_accessor_deterministic.py	`100.00% <ø> (ø)`
xskillscore/tests/test_deterministic.py	`100.00% <ø> (ø)`
xskillscore/tests/test_gridded_metrics.py	`100.00% <ø> (ø)`
xskillscore/tests/test_mask_skipna.py	`100.00% <ø> (ø)`
xskillscore/tests/test_skipna_functionality.py	`100.00% <ø> (ø)`
xskillscore/core/accessor.py	`100.00% <100.00%> (ø)`
xskillscore/core/deterministic.py	`97.69% <100.00%> (+0.15%)`	⬆️
xskillscore/core/np_deterministic.py	`99.23% <100.00%> (+0.05%)`	⬆️
xskillscore/tests/test_metric_results_accurate.py	`100.00% <100.00%> (ø)`
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3e4c49d...6a76bdc. Read the comment docs.

raybellwaves · 2021-05-08T20:41:52Z

@aaronspring anything else before I merge? Planning on a release after this.

aaronspring · 2021-05-08T21:47:45Z

I am fine

xskillscore/core/np_deterministic.py

Co-authored-by: Aaron Spring <aaronspring@users.noreply.github.com>

Ray Bell added 2 commits May 7, 2021 16:59

linslope

5492e04

add eq

f7789e7

raybellwaves requested a review from aaronspring May 7, 2021 21:13

Ray Bell and others added 6 commits May 7, 2021 17:16

pre commit

30b5a9a

rm dup rps

7a0bc40

lint

a97751a

lint again

c14be3f

pin xhistogram

b61a9c6

Merge branch 'main' into add-linslope

f500e1f

raybellwaves mentioned this pull request May 7, 2021

rm weights.load() #306

Merged

aaronspring approved these changes May 7, 2021

View reviewed changes

This comment has been minimized.

Sign in to view

raybellwaves mentioned this pull request May 8, 2021

use built in clip #310

Merged

10 tasks

Merge branch 'xarray-contrib:main' into add-linslope

c6fa65c

raybellwaves closed this May 8, 2021

raybellwaves reopened this May 8, 2021

aaronspring reviewed May 8, 2021

View reviewed changes

xskillscore/core/np_deterministic.py Outdated Show resolved Hide resolved

Update xskillscore/core/np_deterministic.py

6a76bdc

Co-authored-by: Aaron Spring <aaronspring@users.noreply.github.com>

raybellwaves merged commit a2176b5 into xarray-contrib:main May 8, 2021

raybellwaves deleted the add-linslope branch May 8, 2021 23:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

linslope #307

linslope #307

raybellwaves commented May 7, 2021 •

edited

Loading

review-notebook-app bot commented May 7, 2021

pep8speaks commented May 7, 2021 •

edited

Loading

raybellwaves commented May 7, 2021 •

edited

Loading

aaronspring commented May 7, 2021

aaronspring left a comment

raybellwaves commented May 7, 2021

aaronspring commented May 7, 2021

raybellwaves commented May 7, 2021 via email

This comment has been minimized.

This comment has been minimized.

aaronspring commented May 8, 2021

raybellwaves commented May 8, 2021

codecov bot commented May 8, 2021 •

edited

Loading

raybellwaves commented May 8, 2021

aaronspring commented May 8, 2021

linslope #307

linslope #307

Conversation

raybellwaves commented May 7, 2021 • edited Loading

Description

Type of change

How Has This Been Tested?

Checklist (while developing)

Pre-Merge Checklist (final steps)

References

review-notebook-app bot commented May 7, 2021

pep8speaks commented May 7, 2021 • edited Loading

Comment last updated at 2021-05-08 23:44:06 UTC

raybellwaves commented May 7, 2021 • edited Loading

aaronspring commented May 7, 2021

aaronspring left a comment

Choose a reason for hiding this comment

raybellwaves commented May 7, 2021

aaronspring commented May 7, 2021

raybellwaves commented May 7, 2021 via email

This comment has been minimized.

This comment has been minimized.

aaronspring commented May 8, 2021

raybellwaves commented May 8, 2021

codecov bot commented May 8, 2021 • edited Loading

Codecov Report

raybellwaves commented May 8, 2021

aaronspring commented May 8, 2021

raybellwaves commented May 7, 2021 •

edited

Loading

pep8speaks commented May 7, 2021 •

edited

Loading

raybellwaves commented May 7, 2021 •

edited

Loading

codecov bot commented May 8, 2021 •

edited

Loading