Skip to content
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

Dask timeseries prototype #4714

Draft
wants to merge 10 commits into
base: develop
Choose a base branch
from

Conversation

ljwoods2
Copy link
Contributor

@ljwoods2 ljwoods2 commented Sep 23, 2024

Fixes #4713

Changes made in this Pull Request:

  • Added a minimal example "DaskTimeseriesAnalysisBase`
  • Modified the H5MDReader to expose a dasktimeseries method
  • Created example notebook

PR Checklist

  • Tests?
  • Docs?
  • CHANGELOG updated?
  • Issue raised/referenced?

Developers certificate of origin


📚 Documentation preview 📚: https://mdanalysis--4714.org.readthedocs.build/en/4714/

@pep8speaks
Copy link

pep8speaks commented Sep 23, 2024

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

Line 815:58: E252 missing whitespace around parameter equals
Line 815:59: E252 missing whitespace around parameter equals
Line 816:20: E128 continuation line under-indented for visual indent
Line 816:52: E252 missing whitespace around parameter equals
Line 816:53: E252 missing whitespace around parameter equals
Line 817:20: E128 continuation line under-indented for visual indent
Line 817:40: E252 missing whitespace around parameter equals
Line 817:41: E252 missing whitespace around parameter equals
Line 817:66: E252 missing whitespace around parameter equals
Line 817:67: E252 missing whitespace around parameter equals
Line 818:20: E128 continuation line under-indented for visual indent
Line 818:39: E252 missing whitespace around parameter equals
Line 818:40: E252 missing whitespace around parameter equals
Line 819:20: E128 continuation line under-indented for visual indent
Line 819:40: E252 missing whitespace around parameter equals
Line 819:41: E252 missing whitespace around parameter equals
Line 826:80: E501 line too long (81 > 79 characters)
Line 841:80: E501 line too long (113 > 79 characters)
Line 858:1: W293 blank line contains whitespace
Line 862:1: W293 blank line contains whitespace

Comment last updated at 2024-09-23 03:23:40 UTC

@ljwoods2 ljwoods2 marked this pull request as draft September 23, 2024 02:22
Copy link

github-actions bot commented Sep 23, 2024

Linter Bot Results:

Hi @ljwoods2! Thanks for making this PR. We linted your code and found the following:

Some issues were found with the formatting of your code.

Code Location Outcome
main package ⚠️ Possible failure
testsuite ✅ Passed

Please have a look at the darker-main-code and darker-test-code steps here for more details: https://github.com/MDAnalysis/mdanalysis/actions/runs/10987142489/job/30501553782


Please note: The black linter is purely informational, you can safely ignore these outcomes if there are no flake8 failures!

Copy link

codecov bot commented Sep 23, 2024

Codecov Report

Attention: Patch coverage is 5.88235% with 64 lines in your changes missing coverage. Please review.

Project coverage is 92.92%. Comparing base (4fafd51) to head (6d137d1).

Files with missing lines Patch % Lines
package/MDAnalysis/analysis/dasktimeseries.py 0.00% 35 Missing ⚠️
package/MDAnalysis/coordinates/H5MD.py 12.12% 29 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #4714      +/-   ##
===========================================
- Coverage    93.54%   92.92%   -0.63%     
===========================================
  Files          173      174       +1     
  Lines        21434    21502      +68     
  Branches      3981     3987       +6     
===========================================
- Hits         20051    19980      -71     
- Misses         929     1059     +130     
- Partials       454      463       +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expose Dask "lazy timeseries" from compatible readers for full parallelism in analysis
2 participants