Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
distopia 0.4.0 compatibility changes #4734
distopia 0.4.0 compatibility changes #4734
Changes from 23 commits
2ee12e5
9bf0cf7
f8d1295
badc778
e97317b
e0ce653
63162df
0ec2057
2392101
2cb04d2
e4b8a10
517c676
36de301
b49b028
d895d99
5733a74
36c010a
763b493
10356f7
3b0024a
9c149ec
8cae6f8
50efeb9
2c070b7
d4b9e9d
2621552
154d389
c75dce2
02a3d93
07f1438
9101d81
447beac
0a60016
730cee9
a865a46
10e0ec1
195dabe
3f9c2d3
3a7833b
a227494
69c6143
4d2b54a
914f9d3
5711abd
8860d86
b5b788e
26d2665
ba049f9
19f0efc
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Are these versions known? I.e. if we know that this only affects a certain set of releases and they are all <0.3.1, then can we skip the warning altogther and just assume that this only happens in old versions we don't care about?
(intent: I'm trying to completly bypass the follow-up "there are no tests for line 50" with a blunt solution)
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.
We can (now) just say that if we are not able to determine the version then it's too old.
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.
Although we don't need to issue the warning, we need to be able to set the fake distopia version.
Is it worthwhile testing this code path with a proper mock? – @IAlibay ?
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.
Test is simple enough through monkeypatch. Adding it would be great, but I'm not sure it needs to be a blocker for this PR. At the end of the day we'll pin the conda-forge recipe to distopia>=0.4 and probably should do the same for pypi (is there a pypi release of distopia?).
TLDR; I'd be ok to see this is as a follow-up PR, but I agree we probably should do it at some point.
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.
Can we push to seperate issue? Will raise when this is closed.
No there isn't, very hard to ship required libs.
Check warning on line 50 in package/MDAnalysis/lib/_distopia.py
package/MDAnalysis/lib/_distopia.py#L46-L50
Check warning on line 52 in package/MDAnalysis/lib/_distopia.py
package/MDAnalysis/lib/_distopia.py#L52
Check warning on line 66 in package/MDAnalysis/lib/_distopia.py
package/MDAnalysis/lib/_distopia.py#L66
Check warning on line 75 in package/MDAnalysis/lib/_distopia.py
package/MDAnalysis/lib/_distopia.py#L75
Check warning on line 83 in package/MDAnalysis/lib/_distopia.py
package/MDAnalysis/lib/_distopia.py#L83
Check warning on line 89 in package/MDAnalysis/lib/_distopia.py
package/MDAnalysis/lib/_distopia.py#L89
Check warning on line 95 in package/MDAnalysis/lib/_distopia.py
package/MDAnalysis/lib/_distopia.py#L95
Check warning on line 102 in package/MDAnalysis/lib/_distopia.py
package/MDAnalysis/lib/_distopia.py#L102
Check warning on line 108 in package/MDAnalysis/lib/_distopia.py
package/MDAnalysis/lib/_distopia.py#L108
Check warning on line 113 in package/MDAnalysis/lib/_distopia.py
package/MDAnalysis/lib/_distopia.py#L113
Check warning on line 118 in package/MDAnalysis/lib/_distopia.py
package/MDAnalysis/lib/_distopia.py#L118
Check warning on line 123 in package/MDAnalysis/lib/_distopia.py
package/MDAnalysis/lib/_distopia.py#L123
Check warning on line 129 in package/MDAnalysis/lib/_distopia.py
package/MDAnalysis/lib/_distopia.py#L129
Check warning on line 134 in package/MDAnalysis/lib/_distopia.py
package/MDAnalysis/lib/_distopia.py#L134
Check warning on line 139 in package/MDAnalysis/lib/_distopia.py
package/MDAnalysis/lib/_distopia.py#L139
Check warning on line 144 in package/MDAnalysis/lib/_distopia.py
package/MDAnalysis/lib/_distopia.py#L144
Check warning on line 149 in package/MDAnalysis/lib/_distopia.py
package/MDAnalysis/lib/_distopia.py#L149