search

sktime / sktime-dl

DEPRECATED, now in sktime - companion package for deep learning based on TensorFlow
BSD 3-Clause "New" or "Revised" License
596 stars 79 forks source link

Compatibility with sktime v0.6.1 #87

Closed Riyabelle25 closed 3 years ago

Riyabelle25 commented 3 years ago

Reference Issues/PRs

Fixes #74 , #76

What does this implement/fix? Explain your changes.

This PR fixes issues #74 and #76 by updating sktime references in the code to that of sktime v0.6.1. Hence this resolves compatibility issues with sktime v0.6.1, bringing sktime_dl up-to-date with the latest version of sktime. It also updates deprecated Keras code to resolve some of the build failures on running Pytest.

Tested by:

conda activate venv
git clone https://github.com/sktime/sktime-dl.git
cd sktime-dl
pip install  .
python
>> from sktime_dl.deeplearning import CNNClassifier

After this I ran pytest in the root directory, and confirmed that no tests are failing due to incompatibility with sktime. Rather they are associated with version incompatibilities of Keras, Tensorflow, and Numpy, and I debugged by updating deprecated Keras code.

TonyBagnall commented 3 years ago

hi, thanks for this, the travis checks are not currently working so dont worry too much about the errors. @mloning could you review? I'll ask @ABostrom to take a look too

Riyabelle25 commented 3 years ago

Hi, thanks! Currently, there's just the one test failing because of Keras.

mloning commented 3 years ago

@Riyabelle25 as discussed, I suggest adding the pytest decorator to see if this test fails for all networks or only one of them.

ABostrom commented 3 years ago

Going to go through this, today with @jnrusson1, as I'd like to get this PR'd in before dev days next week.

Really great job @Riyabelle25 looks decent.

ABostrom commented 3 years ago

@jnrusson1 noticed a issue and raise this in the #87 issue.

I haven't had a chance to verify this, but @Riyabelle25 if you wouldn't mind taking a closer look that would be very helpful :)

Riyabelle25 commented 3 years ago

I've fixed linting and parametrized the networks to see which fail. Thanks, @ABostrom, @jnrusson1!

Riyabelle25 commented 3 years ago

2021-06-17T07:53:40.4365930Z It seems that sktime cannot be built with OpenMP support. Buildwheels for sktime on macOS build check failed :astonished: Has this happened before?

mloning commented 3 years ago

@Riyabelle25 it shouldn't be necessary to compile sktime as we provide precompiled wheels for it, not sure why it tries to build it from source.