Update maint/build tools #50

mloning · 2020-04-24T15:34:01Z

Reference issues

#28

Summary

Updates binder requirements
Updates GitHub PR/issue templates
Adds linting based on flake8 to check code quality
Adds CI on Azure DevOps
Adds testing/building for macOS and Windows
Adds CHANGELOG.md
Adds Makefile and script for maintenance tasks and release process
Reports coverage and test results on Azure DevOps project website

Comments

For Python 3.6, we haven't released any built wheels for sktime and we need to build them too when we build sktime-dl, which is why we currently require Cython. With the new sktime release, we'll no longer need Cython as a build dependency.

To do

Fix issues with Python 3.6 with TensorFlow 1.9 (see below)
Enforce linting (you can see the report here or pip install and run flake8 in the root directory)

mloning · 2020-04-24T17:35:41Z

For Python 3.6 with TensorFlow 1.9 both the Linux and macOS tests cause a Fatal Python error: Aborted for the MCDCNN classifier. For Linux, this passes on travis. It also passes when I run it locally on macOS.

Thread 0x00007feff2736740 (most recent call first):
File "/usr/share/miniconda/envs/testenv/lib/python3.6/site-packages/tensorflow/python/client/session.py", line 1451 in call
File "/usr/share/miniconda/envs/testenv/lib/python3.6/site-packages/tensorflow/python/keras/backend.py", line 2897 in call
File "/usr/share/miniconda/envs/testenv/lib/python3.6/site-packages/tensorflow/python/keras/engine/training_arrays.py", line 253 in fit_loop
File "/usr/share/miniconda/envs/testenv/lib/python3.6/site-packages/tensorflow/python/keras/engine/training.py", line 1348 in fit
File "/home/vsts/work/1/s/sktime_dl/deeplearning/mcdcnn/_classifier.py", line 172 in fit

Any idea how to fix this?
Do we still want to support Python 3.6 with TensorFlow 1.9?
We could leave them to run on travis

Withington

Looks good. I've not used Azure Pipelines before.

I'd be happy to have sktime-dl support only TensorFlow 2. TF1 depends upon standalone keras and support for that ended in April 2020 .

Can we switch to use only Azure and not both Azure and Travis, to avoid duplication of sections of yml files?

Azure code coverage can't be rendered, it says "Please verify that "Report Directory" containing an HTML report was specified when publishing code coverage."

Withington · 2020-04-25T14:58:52Z

build_tools/azure/test.sh

+set -x  # print executed commands to the terminal
+
+pytest -m="not slow" --verbose --showlocals --durations=20 --junitxml=junit/test-results.xml --cov=sktime_dl --pyargs ../sktime_dl
+


Consider removing "not slow" and running all of the tests.

How long do they need? We could consider always running them on azure?

mloning · 2020-04-25T17:37:27Z

I could refactor the .travis.yml to use the same bash scripts as azure if we want to keep travis for the failing tests or as a back-up. I've seen this before that things work on one CI service but not the other ...
I can try to set up the task to publish coverage results

mloning · 2020-04-26T19:16:51Z

I've figured out where the error comes from: Python 3.6 with TensorFlow 1.9 only works on Ubuntu 16.14 (xenial) and macOS Mojave, but not the latest OS versions. @james-large what's your opinion? Shall we still test these versions?
I've refactored .travis.yml to use the same build and test scripts.
Coverage results are now published on our project page on Azure DevOps.

james-large · 2020-04-28T16:13:09Z

Hmm, certainly don't have to support (or test) tf1.9 by any means. The test was mainly there as a proof that we are backwards compatible that far in case people do still want to use it (for better or worse), but if it varies even by linux versions (let alone windows/macOS), it's likely worth just dropping.

There is a reasonable argument to be made for thoroughly testing most recent tf versions only and leaving it up to the user to figure out if they want to keep older installations, with guidelines in text that as far back as 1.9 is compatible in principle but maybe not in practice

mloning · 2020-04-28T16:16:21Z

Okay, I make sure we test on the most recent TF and OS versions, will push final changes some time later this week.

james-large · 2020-04-28T16:23:14Z

This is good stuff

Looking through the Azure DevOps CI, what (on a high level) are the pros/cons to having both the Azure and Travis checks? On-the-surface redundancy can be useful like you say before, more tests on different platforms can reveal different problems. Assuming they're run in parallel as well then presumably the main cost is just maintenance of two CI systems instead of one?

.binder/requirements.txt

maint_tools/make_release.py

mloning · 2020-04-30T16:11:27Z

Yes, but maintenance of both shouldn't add too much extra work, as they use the same scripts for building and testing. I've added azure dev ops because they offer a better service on their free tier (more parallel builds, unlimited minutes), I also like their syntax and available predefined functionality compared to travis.
travis now runs everything on ubuntu-16.14 (xenial), azure pipelines runs the latest OS versions, and only for TF 1.9 the previous ubunutu and macOS versions.

So, from my side, this PR is good to go.

Once this is merged, I was thinking of opening another PR to clean up any code style violations and enforce PEP8 on azure pipelines, what do you think?

james-large · 2020-05-01T13:21:02Z

Fair dos, yep will merge. I'd be happy with enforcing PEP8, don't know about other people's environments, but for me adhering it it is fine, a couple of old commit have been me just going through and enforcing it on all files via the IDE

Withington · 2020-05-01T13:50:05Z

Yes, happy to work to PEP8.

mloning added 12 commits April 24, 2020 15:30

added binder requirements

71b6562

updated maint/build tools

b536fee

more changes

0a47be4

more changes

1a09e46

more changes

c4a395a

more changes

0ac16ed

more changes

8479fc9

more changes

161e2b2

more changes

f9f86bc

more changes

4c32a1e

more changes

7087053

more changes

3bb4b67

more changes

f06a1cd

mloning added this to the v0.2.0 milestone Apr 24, 2020

mloning linked an issue Apr 24, 2020 that may be closed by this pull request

macOS and Windows environments on travis tests #28

Closed

mloning requested review from james-large and Withington April 25, 2020 11:51

Withington reviewed Apr 25, 2020

View reviewed changes

mloning added 10 commits April 26, 2020 10:33

refactored travis

76d511a

azure publish cov

76a22a4

added cov to pytest cmd

6fba3b8

more changes

f2f84f3

more changes

16feb7b

added Makefile

15877c3

reversed travis changes

87c2a2e

reversed azure shell script changes

154d993

trying out only test script

f4d033d

more changes

a6e9864

mloning added 13 commits April 26, 2020 17:00

more changes

b536a7d

more changes

840dc9b

more changes

adc629a

more changes

e753e40

more changes

02b1532

more changes

424427c

more changes

75affcb

more changes

663ce12

more changes

b94de89

more changes

1f96317

more changes

c231c59

more changes

0ed31b5

more changes

85952ad

more changes

11c66b4

mloning marked this pull request as ready for review April 27, 2020 08:08

mloning commented Apr 29, 2020

View reviewed changes

.binder/requirements.txt Outdated Show resolved Hide resolved

mloning commented Apr 29, 2020

View reviewed changes

maint_tools/make_release.py Outdated Show resolved Hide resolved

Markus Löning added 4 commits April 30, 2020 16:55

Update .binder/requirements.txt

7437fc1

Update maint_tools/make_release.py

ec7edd4

support latest OS versions

20fbc81

more changes

d9c2119

james-large merged commit 9f90a60 into dev May 1, 2020

TonyBagnall deleted the ci branch June 17, 2021 17:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update maint/build tools #50

Update maint/build tools #50

mloning commented Apr 24, 2020 •

edited

Loading

mloning commented Apr 24, 2020 •

edited

Loading

Withington left a comment

Withington Apr 25, 2020

mloning Apr 30, 2020

mloning commented Apr 25, 2020

mloning commented Apr 26, 2020

james-large commented Apr 28, 2020 •

edited

Loading

mloning commented Apr 28, 2020

james-large commented Apr 28, 2020

mloning commented Apr 30, 2020

james-large commented May 1, 2020

Withington commented May 1, 2020

		set -x # print executed commands to the terminal

		pytest -m="not slow" --verbose --showlocals --durations=20 --junitxml=junit/test-results.xml --cov=sktime_dl --pyargs ../sktime_dl

Update maint/build tools #50

Update maint/build tools #50

Conversation

mloning commented Apr 24, 2020 • edited Loading

Reference issues

Summary

Comments

To do

mloning commented Apr 24, 2020 • edited Loading

Withington left a comment

Choose a reason for hiding this comment

Withington Apr 25, 2020

Choose a reason for hiding this comment

mloning Apr 30, 2020

Choose a reason for hiding this comment

mloning commented Apr 25, 2020

mloning commented Apr 26, 2020

james-large commented Apr 28, 2020 • edited Loading

mloning commented Apr 28, 2020

james-large commented Apr 28, 2020

mloning commented Apr 30, 2020

james-large commented May 1, 2020

Withington commented May 1, 2020

mloning commented Apr 24, 2020 •

edited

Loading

mloning commented Apr 24, 2020 •

edited

Loading

james-large commented Apr 28, 2020 •

edited

Loading