Skip to content
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

Switch setup to Poetry #257

Merged
merged 15 commits into from
Jun 24, 2021
Merged

Conversation

adrhill
Copy link
Collaborator

@adrhill adrhill commented Jun 23, 2021

This PR updates the packaging tools, dev dependencies and CI.
The source code remains untouched. Reopened from #254.

Relevant requirements

  • [SWR 1.1.3]: Can be installed via pip.
  • [SWR 3.1.1]: Use and enforcement of codings style.
  • [SWR 3.1.2]: Use of automatic CI pipeline.

Changes

Project structure

  • switch setup to Poetry for packaging and dependency management
    • updated release guide todo_before_release.md
  • use GitHub Actions instead of Travis CI
    • add code style checks with black and isort
    • track code coverage with codecov
  • update project structure
    • use separate tests folder
    • src layout
  • setup dev dependencies, e.g.:
    • pytest config in pyproject.toml
    • isort config in pyproject.toml
    • flake8 config in .flake8
    • pylint config in .pylintrc
  • setup tox in tox.ini
    • run pytest, isort, black, flake8, pylint and try to build docs

Breaking changes

  • remove DeepLIFT
  • update lower bound of dependencies to Python 3.7, TensorFlow 1.15 and Keras 2.3

Currently, I've tagged this as 1.10.0 in the pyproject.toml for the next release, however you might want to use version number 2.0.0 according to the Semantic Versioning convention, as this makes incompatible API changes by removing DeepLIFT.

Reasoning

Poetry

Poetry is a dependency manager and packaging tool that uses PEP 518's pyproject.toml to define dependencies, settings and build systems in a single file.

Poetry automatically resolves dependency conflicts within the specified version constraints and makes it trivial to set up venvs using poetry install. Poetry's poetry.lock ensures everyone working on the package uses consistent package versions.

Building and publishing to PyPI is as easy as

poetry build
poetry publish

Command line dev dependencies like pytest, black and tox can be run using poetry run pytest.

GitHub Actions instead of Travis CI

Travis CI is not free of charge anymore, only giving a limited amount of credits to OSS projects.

Project structure

Using a separate tests folders is considered best pratice in the pytest docs. It is "strongly suggested" to use a src layout so that tools like tox test installed versions of the package instead of local code from the repository.

More discussion can be found here and here.

Dependencies

This PR temporarily uses Keras 2.3 as it is the first and last multi-backend Keras release that supports both TF2 as well as TF1.

Removing DeepLift

As this PR is working towards a release of iNNvestigate for TF2, DeepLift was removed to lighten the workload. The original repo also still only provides TF1 support

@adrhill adrhill requested a review from albermax June 23, 2021 16:48
@codecov-commenter
Copy link

Codecov Report

Merging #257 (78ecae0) into master (d02a79e) will decrease coverage by 60.70%.
The diff coverage is 25.48%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master     #257       +/-   ##
===========================================
- Coverage   82.12%   21.41%   -60.71%     
===========================================
  Files          48       46        -2     
  Lines        4661     4300      -361     
  Branches        0      659      +659     
===========================================
- Hits         3828      921     -2907     
- Misses        833     3357     +2524     
- Partials        0       22       +22     
Impacted Files Coverage Δ
src/innvestigate/__init__.py 0.00% <0.00%> (ø)
src/innvestigate/analyzer/__init__.py 0.00% <0.00%> (ø)
src/innvestigate/analyzer/base.py 0.00% <0.00%> (ø)
src/innvestigate/analyzer/deeptaylor.py 0.00% <0.00%> (ø)
src/innvestigate/analyzer/gradient_based.py 0.00% <0.00%> (ø)
src/innvestigate/analyzer/misc.py 0.00% <0.00%> (ø)
src/innvestigate/analyzer/pattern_based.py 0.00% <0.00%> (ø)
.../innvestigate/analyzer/relevance_based/__init__.py 0.00% <ø> (ø)
...ate/analyzer/relevance_based/relevance_analyzer.py 0.00% <0.00%> (ø)
...stigate/analyzer/relevance_based/relevance_rule.py 0.00% <0.00%> (ø)
... and 75 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 fe1db51...78ecae0. Read the comment docs.

@adrhill
Copy link
Collaborator Author

adrhill commented Jun 23, 2021

I think codecov has been a bit buggy recently.
Some reduction in code coverage is expected and desired, as this PR moves the tests into a separate folder and we don't want to count covered test code.

Running pytest locally, the percentage is now at 72%, not 25%:

---------- coverage: platform darwin, python 3.7.10-final-0 ----------
Name                                                              Stmts   Miss Branch BrPart  Cover
---------------------------------------------------------------------------------------------------
src/innvestigate/__init__.py                                          6      0      0      0   100%
src/innvestigate/analyzer/__init__.py                                21      0      0      0   100%
src/innvestigate/analyzer/base.py                                   312     48    112     14    82%
src/innvestigate/analyzer/deeptaylor.py                              47      2     20      0    97%
src/innvestigate/analyzer/gradient_based.py                         113     13     34      5    88%
src/innvestigate/analyzer/misc.py                                    26      0      8      0   100%
src/innvestigate/analyzer/pattern_based.py                           98      6     28      7    90%
src/innvestigate/analyzer/relevance_based/__init__.py                 1      0      0      0   100%
src/innvestigate/analyzer/relevance_based/relevance_analyzer.py     289     45     87     10    77%
src/innvestigate/analyzer/relevance_based/relevance_rule.py         242     66    148      5    67%
src/innvestigate/analyzer/relevance_based/utils.py                   34     17     18      6    48%
src/innvestigate/analyzer/wrapper.py                                161     10     54     12    89%
src/innvestigate/applications/__init__.py                             0      0      0      0   100%
src/innvestigate/applications/imagenet.py                            74     46     14      0    32%
src/innvestigate/applications/mnist.py                               36     36      6      0     0%
src/innvestigate/layers.py                                          324     60     69     10    79%
src/innvestigate/tools/__init__.py                                    5      0      0      0   100%
src/innvestigate/tools/pattern.py                                   218     33     87     16    78%
src/innvestigate/tools/perturbate.py                                208     48     85     30    68%
src/innvestigate/utils/__init__.py                                   71     30     32      3    49%
src/innvestigate/utils/keras/__init__.py                             24      5     12      1    72%
src/innvestigate/utils/keras/backend.py                              57     31     24      6    40%
src/innvestigate/utils/keras/checks.py                               94     19     30      3    76%
src/innvestigate/utils/keras/graph.py                               498    182    325     37    64%
src/innvestigate/utils/visualizations.py                             60      7     16      7    79%
---------------------------------------------------------------------------------------------------
TOTAL                                                              3019    704   1209    172    72%

@albermax albermax merged commit c3295be into albermax:master Jun 24, 2021
@adrhill adrhill deleted the adrhill/poetry-setup branch June 24, 2021 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants