Skip to content

Prevent double monkey-patching experimental storage#1109

Merged
dwhswenson merged 2 commits into
openpathsampling:masterfrom
dwhswenson:prevent-double-monkeypatch
Jul 30, 2022
Merged

Prevent double monkey-patching experimental storage#1109
dwhswenson merged 2 commits into
openpathsampling:masterfrom
dwhswenson:prevent-double-monkeypatch

Conversation

@dwhswenson

Copy link
Copy Markdown
Member

It was possible to double monkey-patch for the experimental storage. This caused many headaches downstream (e.g., OPS CLI) because that meant that something downstream had to track state of whether OPS had been monkey-patched. It also might be the problem behind #1108. This PR makes it so that monkey-patching is idempotent.

Marked WIP because it still needs tests (which will be slightly a pain), but this is the implementation I'm planning on.

@dwhswenson dwhswenson added bugfix PRs fixing bugs experimental labels Mar 24, 2022
@codecov

codecov Bot commented Mar 24, 2022

Copy link
Copy Markdown

Codecov Report

Merging #1109 (a65264a) into master (9131891) will decrease coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1109      +/-   ##
==========================================
- Coverage   81.76%   81.75%   -0.01%     
==========================================
  Files         142      142              
  Lines       15612    15612              
==========================================
- Hits        12765    12764       -1     
- Misses       2847     2848       +1     
Impacted Files Coverage Δ
openpathsampling/netcdfplus/cache.py 63.72% <0.00%> (-0.33%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9131891...a65264a. Read the comment docs.

includes tests for double patching and for unpatching
@dwhswenson dwhswenson marked this pull request as ready for review July 25, 2022 22:49
@dwhswenson dwhswenson changed the title [WIP] Prevent double monkey-patching experimental storage Prevent double monkey-patching experimental storage Jul 25, 2022
@dwhswenson

Copy link
Copy Markdown
Member Author

Tests weren't quite as bad as I feared, once I sat down to write them.

This is ready for review and comment. I will leave it open for at least 48 hours, merging no earlier than Thu 28 Jul 00:00 GMT (Wed 27 Jul 19:00 my local).

@sroet sroet left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A couple comments, nothing mayor, so feel free to ignore if you have a different opinion

Comment thread openpathsampling/experimental/storage/monkey_patches.py
Comment thread openpathsampling/experimental/storage/test_monkey_patches.py
Comment thread openpathsampling/experimental/storage/test_monkey_patches.py
@sroet

sroet commented Jul 27, 2022

Copy link
Copy Markdown
Member

All my comments have been handled (all of them where due to me misremembering how python does things). This LGTM, feel free to merge

@dwhswenson dwhswenson merged commit e566186 into openpathsampling:master Jul 30, 2022
@dwhswenson dwhswenson deleted the prevent-double-monkeypatch branch July 30, 2022 02:46
@dwhswenson dwhswenson mentioned this pull request Jan 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix PRs fixing bugs experimental

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants