-
Notifications
You must be signed in to change notification settings - Fork 7
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
Allow dask #181
Allow dask #181
Conversation
…qual test_correlation and test_correlation_with_dim
…qual test_correlation and test_correlation_with_dim
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Just two small comments
CITATION.cff
Outdated
@@ -38,6 +38,12 @@ authors: | |||
given-names: Jannes | |||
affiliation: "Vrije Universiteit Amsterdam" | |||
|
|||
- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- | |
- |
the other dashes are less indented. not sure if it matters
@@ -35,6 +35,7 @@ classifiers = [ | |||
"Programming Language :: Python :: 3.11", | |||
] | |||
dependencies = [ | |||
"dask", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it required as dependency? The code runs fine without dask right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, I'll check it without and see if it still works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added dask back in as the tests broke on github actions after removing, so perhaps it is needed.
Continuing on from work from #176, making sure tests and formatting pass. Currently sonarcloud test is failing, probably due to changes on their end (issue #182), which we plan to fix in a future pull request.