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

T020: update deprecated code fix broken ci #363

Conversation

dominiquesydow
Copy link
Collaborator

@dominiquesydow dominiquesydow commented May 22, 2023

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

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@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
2 of 6 checks passed
@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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants