Port GH actions and other fixes to master #3061

lilyminium · 2020-12-08T00:01:47Z

In aid of #2768.

Possibly this will help fix the last remaining obstacle, #3004?

Added all commits to develop that did not involve a .. versionchanged:: 2.0.0 docstring. Please let me know if any were premature.

PR Checklist

Tests?
Docs?
CHANGELOG updated?
Issue raised/referenced?

Fixes MDAnalysis#3036 ## Work done in this PR - Adds a continuous integration workflow based on github actions. - Disables previous continuous integration workflow based on TravisCI. - Fixes minor Visibledeprecation warnings with the hole2 tests.

Fixes MDAnalysis#3046. Deploys docs in gh-ci.yaml script and removes Travis

* TST, CI: add ARM64 Graviton 2 to CI * start testing MDAnalysis on ARM64 using new AWS Graviton 2 architecture available in Travis CI * testing on my fork shows only two failures in the MDAnalysis test suite using a fairly minimal set of dependencies; we can probably either fix those or skip them and open an issue * the test failures are: `test_written_remarks_property` `TestEncoreClustering.test_clustering_three_ensembles_two_identical` * run time for this CI entry is about 23 minutes, which is solid for ARM; we save a lot of time using experimental upstream wheels in some cases and excluding (source builds of) dependencies where possible until the ARM64 binary wheel ecosystem matures a bit more * MAINT: ARM64 shims * adjust `CAffinityPropagation()` C-level function to use `double` instead of `float` in some strategic locations that allow the `test_clustering_three_ensembles_two_identical()` test to pass on gcc115 ARM64 machine * mark `test_written_remarks_property()` as a known failure on ARM64; it fails in Travis CI on that platform, but not on gcc115 ARM64 machine; that said, this test is already known to be flaky on Windows 32-bit (may depend on character settings on machine?) * MAINT: MR 2956 revisions * reduce optimization level of MDAnalysis builds on ARM64 to avoid problems with `ap.c` causing test failures on that architecture * revert any source changes to `ap.c`

.github/workflows/gh-ci.yaml

Fixes MDAnalysis#2993 Redirect URLs in stable/sitemap.xml and dev/sitemap.xml to correct locations

…2953) * fix NameError when catching NoDataError * add test + missing imports

pep8speaks · 2020-12-08T00:20:09Z

Hello @lilyminium! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

In the file package/setup.py:

Line 275:80: E501 line too long (81 > 79 characters)
Line 278:80: E501 line too long (81 > 79 characters)

In the file testsuite/MDAnalysisTests/formats/test_libdcd.py:

Line 323:21: E127 continuation line over-indented for visual indent

Comment last updated at 2020-12-08 22:23:00 UTC

…DAnalysis#3029) Fixes MDAnalysis#3028 * Improves the performance of the ParmEd converter by using a dictionary lookup for the atomgroup to universe index mapping.

codecov · 2020-12-08T04:59:44Z

Codecov Report

Merging #3061 (e79c80c) into master (2ef90f2) will increase coverage by 0.91%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #3061      +/-   ##
==========================================
+ Coverage   91.46%   92.38%   +0.91%     
==========================================
  Files         173      181       +8     
  Lines       24203    24755     +552     
  Branches     3193     3194       +1     
==========================================
+ Hits        22138    22870     +732     
- Misses       1438     1789     +351     
+ Partials      627       96     -531

Impacted Files	Coverage Δ
package/MDAnalysis/coordinates/ParmEd.py	`88.26% <100.00%> (+4.83%)`	⬆️
package/MDAnalysis/topology/tpr/obj.py	`96.82% <0.00%> (-3.18%)`	⬇️
...onality_reduction/DimensionalityReductionMethod.py	`97.14% <0.00%> (-2.86%)`	⬇️
package/MDAnalysis/coordinates/chemfiles.py	`90.47% <0.00%> (-1.20%)`	⬇️
package/MDAnalysis/analysis/base.py	`100.00% <0.00%> (ø)`
package/MDAnalysis/tests/__init__.py	`100.00% <0.00%> (ø)`
package/MDAnalysis/tests/datafiles.py	`31.25% <0.00%> (ø)`
package/MDAnalysis/analysis/encore/cutils.pyx	`100.00% <0.00%> (ø)`
package/MDAnalysis/analysis/legacy/__init__.py	`0.00% <0.00%> (ø)`
package/MDAnalysis/due.py	`56.09% <0.00%> (ø)`
... and 93 more

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 2ef90f2...e79c80c. Read the comment docs.

lilyminium · 2020-12-08T09:33:30Z

The current thing I'm working on is that python 2.7 takes forever to install dependencies -- I haven't let it time out yet but it's been 1 hr+ stuck at that step. Not sure why because everything installs fine on my own device.

lilyminium · 2020-12-08T22:36:55Z

.github/workflows/gh-ci.yaml

+  MDA_CONDA_MIN_DEPS: "pip pytest mmtf-python biopython networkx cython matplotlib scipy griddataformats hypothesis gsd codecov"
+  MDA_CONDA_EXTRA_DEPS: "seaborn"
+  MDA_CONDA_PY3_DEPS: "netcdf4 scikit-learn joblib>=0.12 clustalw=2.1 chemfiles mock"
+  MDA_CONDA_PY27_DEPS: "clustalw=2.1 netcdf4=1.3.1 six=1.15.0 funcsigs<=1.0.2 mock<=3.0.5"
+  MDA_CONDA_PY35_DEPS: "netcdf4 scikit-learn joblib>=0.12 clustalw=2.1 mock<=3.0.5"
+  MDA_PIP_MIN_DEPS: 'coveralls coverage<5 pytest-cov pytest-xdist'
+  MDA_PIP_EXTRA_DEPS: 'duecredit parmed'
+  MDA_PIP_PY27_DEPS: "setuptools<45.0.0 chemfiles scikit-learn joblib>=0.12"
+  MDA_PIP_PY3_DEPS: ""
+  MDA_PIP_PY35_DEPS: ""


This and adding the 2.7 and 3.5 jobs are the primary changes I made. Py3.5 and 2.7 seemed to require a lot of pinning in gh actions for conda to resolve dependencies.

If we have to pin for versions, should this be reflected in our setup (or maybe it is)? We probably want to avoid further matplotlib-like issues in 1.0.1?

So conda never refused to resolve dependencies in the 1-2 hours I would let it run. In addition, on both my linux and mac machines it resolved the original unpinned versions easily within a couple minutes. Moreover, Travis didn't require pinning. I'm more inclined to think that this is a GH Action quirk than anything else. On the other hand, seeing as we know those versions work, I am up for it.

To be honest this sounds more like miniconda trickery more than GH issues :/ I don't know, I guess we can't release a new version that goes beyond tested limits.

lilyminium · 2020-12-08T22:38:19Z

.github/workflows/gh-ci.yaml

+              codecov: true
+              numpy: numpy=1.16
+              extra_deps: "EXTRA PY27"
+              conda-version: 4.7.10


Pinning conda didn't really help in resolving dependencies but I found astropy's reason for pinning persuasive (conda is not always so stable from update to update)

Given that the old CI behaviour was just to pin for everything, wouldn't it be cleaner to enforce this as a blanket thing for the entire matrix?

I think it broke for a 3.7 job?

How odd, is this the same version that the ci helpers were pinning to?

lilyminium · 2020-12-08T22:39:44Z

.github/workflows/gh-ci.yaml

+          conda_deps="$conda_deps ${!conda_var}"
+          echo $conda_deps
+          pip_deps="$pip_deps ${!pip_var}"
+        done


I changed this part from @IAlibay's original setup because setting up all the environment variables was a little bit of a pain. I am however happy to go back to the original construct now that we've figured out which dependencies to list.

I don't particularly think we should push for keeping things the same as what they are on develop (they are both completely different kettles of fish).

That being said, at least to me, the dependency list is getting quite bloated/hard to follow.

As a thought here, is there maybe a way we could improve readability/maintainability by splitting up the job into multiple versions of it? It would mean a lot of duplication, but if it comes with increased readability, I personally think it might be a good tradeoff.

Could do -- we could also have environment.yml files for pythons 2.7, 3.5, fully featured, and minimal. That also saves users from having to work this stuff out.

So the reason I chose to avoid env files in develop is that they cause really big slowdowns for some reason, conda setup time was going from ~ 3 mins to ~ 15 mins. That and mamba 100% hates pip in environment files.

I'd prefer stay away from them on develop, but if we need them to clean up the matrix here, I don't have any objections (as long as CI actually completes).

IAlibay

Thanks for working on this @lilyminium

I have a few comments (particularly re: aarch64 inclusion), but I don't think any of them are blocking so I'll set it to approve.

IAlibay · 2020-12-08T22:46:06Z

.disabled-travis.yml

@@ -111,6 +111,31 @@ jobs:
           INSTALL_HOLE="false"
           CODECOV="true" SETUP_CMD="${PYTEST_FLAGS} --cov=MDAnalysis"

+    - os: linux


Are we really planning to bring aarch64 support to 1.0.x? Given it's still experimental (and I think it requires as-of-yet unreleased numpy versions), maybe it's best to leave it out for now at least?

Perhaps @tylerjereddy has a better view of whether it fits in a near-future 1.0.1 release?

IAlibay · 2020-12-08T22:47:51Z

.github/workflows/gh-ci.yaml

+  MDA_CONDA_MIN_DEPS: "pip pytest mmtf-python biopython networkx cython matplotlib scipy griddataformats hypothesis gsd codecov"
+  MDA_CONDA_EXTRA_DEPS: "seaborn"
+  MDA_CONDA_PY3_DEPS: "netcdf4 scikit-learn joblib>=0.12 clustalw=2.1 chemfiles mock"
+  MDA_CONDA_PY27_DEPS: "clustalw=2.1 netcdf4=1.3.1 six=1.15.0 funcsigs<=1.0.2 mock<=3.0.5"
+  MDA_CONDA_PY35_DEPS: "netcdf4 scikit-learn joblib>=0.12 clustalw=2.1 mock<=3.0.5"
+  MDA_PIP_MIN_DEPS: 'coveralls coverage<5 pytest-cov pytest-xdist'
+  MDA_PIP_EXTRA_DEPS: 'duecredit parmed'
+  MDA_PIP_PY27_DEPS: "setuptools<45.0.0 chemfiles scikit-learn joblib>=0.12"
+  MDA_PIP_PY3_DEPS: ""
+  MDA_PIP_PY35_DEPS: ""


If we have to pin for versions, should this be reflected in our setup (or maybe it is)? We probably want to avoid further matplotlib-like issues in 1.0.1?

IAlibay · 2020-12-08T22:49:46Z

.github/workflows/gh-ci.yaml

+              codecov: true
+              numpy: numpy=1.16
+              extra_deps: "EXTRA PY27"
+              conda-version: 4.7.10


Given that the old CI behaviour was just to pin for everything, wouldn't it be cleaner to enforce this as a blanket thing for the entire matrix?

IAlibay · 2020-12-08T22:51:28Z

.github/workflows/gh-ci.yaml

+            - name: minimal-ubuntu
+              os: ubuntu-latest
+              python-version: 3.6
+              extra_deps: ""


This has an empty extra_deps, but macOS doesn't? Can we get away with nothing here?

IAlibay · 2020-12-08T22:56:28Z

.github/workflows/gh-ci.yaml

+          conda_deps="$conda_deps ${!conda_var}"
+          echo $conda_deps
+          pip_deps="$pip_deps ${!pip_var}"
+        done


I don't particularly think we should push for keeping things the same as what they are on develop (they are both completely different kettles of fish).

That being said, at least to me, the dependency list is getting quite bloated/hard to follow.

As a thought here, is there maybe a way we could improve readability/maintainability by splitting up the job into multiple versions of it? It would mean a lot of duplication, but if it comes with increased readability, I personally think it might be a good tradeoff.

lilyminium · 2020-12-22T00:23:12Z

Superseded by #3067.

IAlibay and others added 3 commits December 8, 2020 10:58

Deploy docs via GitHub Actions CI (MDAnalysis#3053)

05e15b9

Fixes MDAnalysis#3046. Deploys docs in gh-ci.yaml script and removes Travis

IAlibay reviewed Dec 8, 2020

View reviewed changes

.github/workflows/gh-ci.yaml Show resolved Hide resolved

lilyminium and others added 2 commits December 8, 2020 11:10

Re-write URLs in sitemap.xml (MDAnalysis#2994)

de022c4

Fixes MDAnalysis#2993 Redirect URLs in stable/sitemap.xml and dev/sitemap.xml to correct locations

ParmEdConverter: fix NameError when catching NoDataError (MDAnalysis#…

8851a2e

…2953) * fix NameError when catching NoDataError * add test + missing imports

lilyminium force-pushed the gh-actions-master branch 5 times, most recently from 2b65941 to 43e964c Compare December 8, 2020 02:51

Improve the performance of ParmEd converter. (Fix MDAnalysis#3028) (M…

218022a

…DAnalysis#3029) Fixes MDAnalysis#3028 * Improves the performance of the ParmEd converter by using a dictionary lookup for the atomgroup to universe index mapping.

lilyminium force-pushed the gh-actions-master branch from a5c97a8 to 590c180 Compare December 8, 2020 03:48

lilyminium force-pushed the gh-actions-master branch 3 times, most recently from 426664a to 3574cf8 Compare December 8, 2020 09:09

attempt py2...

e79c80c

lilyminium force-pushed the gh-actions-master branch from e2df870 to e79c80c Compare December 8, 2020 22:22

lilyminium requested review from IAlibay and orbeckst December 8, 2020 22:34

lilyminium commented Dec 8, 2020

View reviewed changes

IAlibay approved these changes Dec 8, 2020

View reviewed changes

lilyminium mentioned this pull request Dec 9, 2020

Run GH Actions CI for py3.6+ and keep Travis for 3.5 and 2.7 #3067

Merged

4 tasks

lilyminium closed this Dec 22, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Port GH actions and other fixes to master #3061

Port GH actions and other fixes to master #3061

lilyminium commented Dec 8, 2020 •

edited

Loading

pep8speaks commented Dec 8, 2020 •

edited

Loading

codecov bot commented Dec 8, 2020 •

edited

Loading

lilyminium commented Dec 8, 2020

lilyminium Dec 8, 2020

IAlibay Dec 8, 2020

lilyminium Dec 8, 2020

IAlibay Dec 8, 2020

lilyminium Dec 8, 2020

IAlibay Dec 8, 2020

lilyminium Dec 8, 2020

IAlibay Dec 8, 2020

lilyminium Dec 8, 2020

IAlibay Dec 8, 2020

lilyminium Dec 8, 2020

IAlibay Dec 8, 2020

IAlibay left a comment

IAlibay Dec 8, 2020

IAlibay Dec 8, 2020

IAlibay Dec 8, 2020

IAlibay Dec 8, 2020

IAlibay Dec 8, 2020

IAlibay Dec 8, 2020

lilyminium commented Dec 22, 2020

Port GH actions and other fixes to master #3061

Port GH actions and other fixes to master #3061

Conversation

lilyminium commented Dec 8, 2020 • edited Loading

PR Checklist

pep8speaks commented Dec 8, 2020 • edited Loading

Comment last updated at 2020-12-08 22:23:00 UTC

codecov bot commented Dec 8, 2020 • edited Loading

Codecov Report

lilyminium commented Dec 8, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IAlibay left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lilyminium commented Dec 22, 2020

lilyminium commented Dec 8, 2020 •

edited

Loading

pep8speaks commented Dec 8, 2020 •

edited

Loading

codecov bot commented Dec 8, 2020 •

edited

Loading