-
Notifications
You must be signed in to change notification settings - Fork 667
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
AuxReader for EDR Files #3749
AuxReader for EDR Files #3749
Changes from 58 commits
f24d296
0906225
a7c1a81
485b5d8
1048271
0b49bad
e0767b4
0f7b1af
4f944b3
ca1a014
54022bf
a7f27b1
ca39385
6159f2d
f228aa7
73143f6
3be01c6
46584ab
0733647
77d8fa1
3105f0e
b2f30e3
f48c74c
1f921ea
9152604
f59e6fa
b29747c
3916bdf
0092d34
9235600
08907c3
e78ab9e
0119ee1
c233970
cccc1fd
aa226ca
d77517b
fed3b06
ed20993
cea6a97
3660770
1a0f4e3
117d138
c7bde96
55a794a
5e7a779
dfaba25
d1892f6
c5a3a7c
2cc8390
88c4651
8b71850
d599e97
e8f5998
336cc80
a62066e
0ad0079
064ab37
e256498
8d9e077
11c3924
e3a2bb8
828b2dc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -76,6 +76,8 @@ inputs: | |||||||||||||||||||||||||||||||
default: 'pytest-cov<=2.10.1' | ||||||||||||||||||||||||||||||||
pytest-xdist: | ||||||||||||||||||||||||||||||||
default: 'pytest-xdist' | ||||||||||||||||||||||||||||||||
pyedr: | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This also needs adding to the azure pipelines file: mdanalysis/azure-pipelines.yml Lines 84 to 98 in 2c32ed1
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We are not at this point making pyedr a core dependency - going by the fact that we aren't touching setup.py etc... (I would like to get more coredev oks on this anyways) As a result, I would ask that this be moved to the pip optional dependencies section. |
||||||||||||||||||||||||||||||||
default: 'pyedr' | ||||||||||||||||||||||||||||||||
hmacdope marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||
# pip-install optional dependencies | ||||||||||||||||||||||||||||||||
duecredit: | ||||||||||||||||||||||||||||||||
default: 'duecredit' | ||||||||||||||||||||||||||||||||
|
@@ -132,14 +134,15 @@ runs: | |||||||||||||||||||||||||||||||
else | ||||||||||||||||||||||||||||||||
conda install ${CONDA_DEPS} | ||||||||||||||||||||||||||||||||
fi | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
- name: pip_install | ||||||||||||||||||||||||||||||||
shell: bash -l {0} | ||||||||||||||||||||||||||||||||
env: | ||||||||||||||||||||||||||||||||
PIP_MIN_DEPS: | | ||||||||||||||||||||||||||||||||
${{ inputs.coverage }} | ||||||||||||||||||||||||||||||||
${{ inputs.pytest-cov }} | ||||||||||||||||||||||||||||||||
${{ inputs.pytest-xdist }} | ||||||||||||||||||||||||||||||||
${{ inputs.pyedr }} | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As above, optional dep please. |
||||||||||||||||||||||||||||||||
PIP_OPT_DEPS: | | ||||||||||||||||||||||||||||||||
${{ inputs.duecredit }} | ||||||||||||||||||||||||||||||||
${{ inputs.parmed }} | ||||||||||||||||||||||||||||||||
|
@@ -150,7 +153,6 @@ runs: | |||||||||||||||||||||||||||||||
else | ||||||||||||||||||||||||||||||||
PIP_DEPS="${PIP_MIN_DEPS} ${{ inputs.extra-pip-deps }}" | ||||||||||||||||||||||||||||||||
fi | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
# do the install | ||||||||||||||||||||||||||||||||
pip install ${PIP_DEPS} | ||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -96,6 +96,7 @@ jobs: | |
tqdm | ||
threadpoolctl | ||
fasteners | ||
pyedr | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not common practice to have optional deps in azure pipeline runners, I'd remove it for now. |
||
displayName: 'Install dependencies' | ||
# for wheel install testing, we pin to an | ||
# older NumPy, the oldest version we support and that | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,7 +13,8 @@ The rules for this file: | |
* release numbers follow "Semantic Versioning" http://semver.org | ||
|
||
------------------------------------------------------------------------------ | ||
??/??/?? IAlibay, Luthaf, hmacdope, rafaelpap, jbarnoud | ||
|
||
??/??/?? IAlibay, Luthaf, hmacdope, rafaelpap, jbarnoud, BFedder | ||
|
||
* 2.4.0 | ||
|
||
|
@@ -22,10 +23,21 @@ Fixes | |
* Upgrade to chemfiles 0.10.3 (Issue #3798) | ||
|
||
Enhancements | ||
* MDAnalysis.units now has a lookup dictionary called MDANALYSIS_BASE_UNITS | ||
where unit types like length or speed are mapped to their base units in | ||
MDAnalysis (i.e. "A" or "A/ps") | ||
* A new AuxReader for the GROMACS EDR file format was implemented. | ||
orbeckst marked this conversation as resolved.
Show resolved
Hide resolved
|
||
(Issue # 3629, PR # 3749) | ||
* Added benchmarks for bonds topology attribute (Issue #3785, PR #3806) | ||
|
||
|
||
Changes | ||
* `MDAnalysis.coordinates.base.add_auxiliary()` now supports adding of | ||
multiple auxiliary data points at the same time. To this end, the API was | ||
rewritten and now expects dictionaries of `desired attribute name`: | ||
`name in auxiliary data file` as input. Adding all data found in the | ||
auxiliary data source is possible by omitting the dictionary and explicitly | ||
passing the AuxReader instance as `auxdata`. (PR #3749) | ||
IAlibay marked this conversation as resolved.
Show resolved
Hide resolved
|
||
* The "AMBER" entry in setup.py's extra_requires has now been removed. Please | ||
use `pip install ./package[extra_formats]` instead (PR #3810) | ||
* `Universe.empty` emmits less warnings (PR #3814) | ||
|
@@ -35,7 +47,6 @@ Deprecations | |
|
||
08/29/22 IAlibay, PicoCentauri, orbeckst, hmacdope, rmeli, miss77jun, rzhao271, | ||
yuxuanzhuang, hsadia538, lilyminium | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't remove this. |
||
* 2.3.0 | ||
|
||
Fixes | ||
|
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 we making pyedr an optional dependency? I'm somewhat ok either way, if we could drop the 14MB test files in the next release it's small enough that it could be core if we wanted to. Thoughts @MDAnalysis/coredevs ?
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.
In any case, pyedr needs to go into the
setup.py
either as an extra_requires (if not a core dependency), or just as an install_requires otherwiseThere 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.
If we can decrease the size to <1 MB then it's just more convenient to have it as a core dependency — it's light weight and we maintain it.