Skip to content

T020: update deprecated code fix broken ci #363

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
May 23, 2023
Merged

T020: update deprecated code fix broken ci #363

merged 6 commits into from
May 23, 2023

Conversation

dominiquesydow
Copy link
Collaborator

@dominiquesydow dominiquesydow commented May 22, 2023 •
edited
Loading

Description

T020 fails with cryptic message in CI under MacOS:

UserWarning: Reload offsets from trajectory
 ctime or size or n_atoms did not match
  warnings.warn("Reload offsets from trajectory\n "

Try to fix this by rerunning on local MacOS & fix warnings etc. along the way.

Note: I recommend merging #362 first, only then this PR (as this PR branches off t019-exclude-windows-from-ci).

Todos

  • Fix code with deprecation warnings from MDAnalysis
    • Instance 1 
      rmsd_analysis = rms.RMSD(...)
rmsd_analysis.run()
rmsd_analysis.rmsd → rmsd_analysis.results.rmsd
    • Instance 2 
      pairwise_rmsd = diffusionmap.DistanceMatrix(...)
pairwise_rmsd.run()
pairwise_rmsd.dist_matrix → pairwise_rmsd.results.dist_matrix
    • Instance 3 
      hbonds = HBA(...)
hbonds.run()
hbonds.hbonds → hbonds.results.hbonds
  • Rerun notebooks manually (take care of 3D viewer views)
  • Update xtc_offsets.npz file and add xtc_offsets.lock (from manual rerun), see notes in docs --> CI fail remains
  • Remove both files, see commit f2878ee --> CI fail gone? yes, it seems so (passing CI)
  • Run CI only for T020 (to be faster) & revert back those changes from commit eb45383

Status

  • Ready to go

@dominiquesydow dominiquesydow self-assigned this May 22, 2023
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

@dominiquesydow dominiquesydow mentioned this pull request May 22, 2023
Merged
16 tasks
@dominiquesydow
Copy link
Collaborator Author

@mbackenkoehler, this PR should be ready as well - I think the problem was the xtc offset file, I removed it now & also updated the notebooks that showed a couple of deprecation warnings.
I just kicked off the CI on all notebooks (minus T019 on Windows).

I recommend merging first this PR:
#362
and then this PR will update automatically asking to merge on dev (at the moment asking to merge onto 019-exclude-windows-from-ci), which you should then be able to do without conflicts.

@mbackenkoehler
Copy link
Collaborator

I re-ran the test on dev and now - for some reason... - T018 is failing as well. So this is clearly not the culprit ;)

@mbackenkoehler mbackenkoehler merged commit d925e25 into t019-exclude-windows-from-ci May 23, 2023
@mbackenkoehler mbackenkoehler deleted the t020-update-deprecated-code-fix-broken-ci branch May 23, 2023 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mbackenkoehler mbackenkoehler Awaiting requested review from mbackenkoehler

Assignees

@dominiquesydow dominiquesydow

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@dominiquesydow @mbackenkoehler