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

[tests/bug] improve tests and fix a minor bug #1229

Merged
merged 9 commits into from
Jun 27, 2023

Conversation

felixdittrich92
Copy link
Contributor

@felixdittrich92 felixdittrich92 commented Jun 24, 2023

This PR:

  • updates codecov action to latest
  • updates / adds some missing tests (there are still some missing ones but this would go beyond the scope of this PR)
  • fixes a minor bug in merge_strings -> doctr/models/recognition/utils.py
  • add some no-cover for file_utils.py and hub.py because we cannot test some cases or it is already tested by third party lib (e.g.: hf hub)

@felixdittrich92 felixdittrich92 added type: enhancement Improvement module: models Related to doctr.models ext: tests Related to tests folder framework: pytorch Related to PyTorch backend framework: tensorflow Related to TensorFlow backend labels Jun 24, 2023
@felixdittrich92 felixdittrich92 added this to the 0.6.1 milestone Jun 24, 2023
@felixdittrich92 felixdittrich92 self-assigned this Jun 24, 2023
@codecov
Copy link

codecov bot commented Jun 24, 2023

Codecov Report

❗ No coverage uploaded for pull request base (main@61a32a1). Click here to learn what that means.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1229   +/-   ##
=======================================
  Coverage        ?   95.65%           
=======================================
  Files           ?      154           
  Lines           ?     6876           
  Branches        ?        0           
=======================================
  Hits            ?     6577           
  Misses          ?      299           
  Partials        ?        0           
Flag Coverage Δ
unittests 95.65% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
doctr/file_utils.py 87.80% <ø> (ø)
doctr/models/factory/hub.py 100.00% <ø> (ø)
doctr/io/image/tensorflow.py 100.00% <100.00%> (ø)
doctr/models/recognition/utils.py 100.00% <100.00%> (ø)

@felixdittrich92 felixdittrich92 changed the title [tests] improve tests [tests/bug] improve tests and fix a minor bug Jun 24, 2023
@felixdittrich92 felixdittrich92 marked this pull request as ready for review June 26, 2023 06:57
doctr/models/recognition/utils.py Show resolved Hide resolved
tests/common/test_core.py Outdated Show resolved Hide resolved
tests/common/test_datasets_utils.py Outdated Show resolved Hide resolved
tests/tensorflow/test_models_detection_tf.py Outdated Show resolved Hide resolved
tests/tensorflow/test_models_recognition_tf.py Outdated Show resolved Hide resolved
import doctr


def test_version():
assert len(doctr.__version__.split(".")) == 3


@pytest.mark.skipif(doctr.is_torch_available() and doctr.is_tf_available(), reason="torch and tf are available")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@odulcy-mindee this is better it skips the tests (make tests works now also if both backends installed if you develop on doctr) and does not effect the CI jobs

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah, that's even better

Comment on lines +19 to +30
["db_resnet34", (3, 512, 512), (1, 512, 512), True, True],
["db_resnet34", (3, 512, 512), (1, 512, 512), True, False],
["db_resnet50", (3, 512, 512), (1, 512, 512), True, True],
["db_resnet50", (3, 512, 512), (1, 512, 512), True, False],
["db_mobilenet_v3_large", (3, 512, 512), (1, 512, 512), True, True],
["db_mobilenet_v3_large", (3, 512, 512), (1, 512, 512), True, False],
["linknet_resnet18", (3, 512, 512), (1, 512, 512), True, True],
["linknet_resnet18", (3, 512, 512), (1, 512, 512), True, False],
["linknet_resnet34", (3, 512, 512), (1, 512, 512), True, True],
["linknet_resnet34", (3, 512, 512), (1, 512, 512), True, False],
["linknet_resnet50", (3, 512, 512), (1, 512, 512), True, True],
["linknet_resnet50", (3, 512, 512), (1, 512, 512), True, False],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just discovered we can stack parametrize parameter from documentation (quite useful for train_mode argument):

import pytest


@pytest.mark.parametrize("x", [0, 1])
@pytest.mark.parametrize("y", [2, 3])
def test_foo(x, y):
    pass

We can keep it like that for this PR, that's fine 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah nice good to know 👍
Yes, I also think that we can slim it down in another PR :)

@felixdittrich92 felixdittrich92 merged commit 863a2e0 into mindee:main Jun 27, 2023
@felixdittrich92 felixdittrich92 deleted the improved-tests branch June 27, 2023 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ext: tests Related to tests folder framework: pytorch Related to PyTorch backend framework: tensorflow Related to TensorFlow backend module: models Related to doctr.models type: enhancement Improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants