Skip to content

fixed bug in dask.array.roll() for roll-shifts the size of the input array#8723

Merged
jcrist merged 3 commits intodask:mainfrom
ParticularMiner:roll_bug
Feb 15, 2022
Merged

fixed bug in dask.array.roll() for roll-shifts the size of the input array#8723
jcrist merged 3 commits intodask:mainfrom
ParticularMiner:roll_bug

Conversation

@ParticularMiner
Copy link
Copy Markdown
Contributor

@ParticularMiner ParticularMiner commented Feb 15, 2022 •
edited
Loading

  • Closes (not applicable)
  • Tests added / passed
  • Passes pre-commit run --all-files

Discovered a bug while working with dask.array.roll():

x = da.arange(2, 3)
y = da.roll(x, 1)
y[0] = 0
x.compute(), y.compute()

Output:

(array([0]), array([0]))

Should have been:

(array([2]), array([0]))

@github-actions github-actions bot added the array label Feb 15, 2022
@ParticularMiner ParticularMiner changed the title fixed bug in dask.array.concatenate() for unit-length sequence of arrays fixed bug in dask.array.roll() for roll-shifts the size of the input array Feb 15, 2022
jcrist
jcrist reviewed Feb 15, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@jcrist jcrist left a comment

Choose a reason for hiding this comment

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

Thanks for finding this bug @ParticularMiner.

@jcrist
Copy link
Copy Markdown
Member

jcrist commented Feb 15, 2022

Also - sidenote - it looks like you've locked your repo for maintainers to make changes to your PRs (e.g. I can't apply my suggestions above myself). If this is intentional that's fine, but if not, in the future disabling this would let us make small fixes like this without requiring you to make those changes yourself.

@ParticularMiner
Copy link
Copy Markdown
Contributor Author

ParticularMiner commented Feb 15, 2022

Oh I see. I was unsure whether this feature was useful or not. But now that you've mentioned it I'll refrain from disabling the feature in future.

Thanks @jcrist

@jcrist jcrist merged commit dca1039 into dask:main Feb 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jcrist jcrist jcrist approved these changes

Assignees

No one assigned

Labels

array

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ParticularMiner @jcrist