Skip to content

Assume if TYPE_CHECKING: ... else: ... block is covered #831

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

Open
Tracked by #131
retnikt opened this issue Aug 2, 2019 · 12 comments
Open
Tracked by #131

Assume if TYPE_CHECKING: ... else: ... block is covered #831

retnikt opened this issue Aug 2, 2019 · 12 comments
Labels
bug Something isn't working

Comments

@retnikt
Copy link

retnikt commented Aug 2, 2019

if typing.TYPE_CHECKING blocks are never run but are still counted as needing test coverage.
( https://docs.python.org/3.7/library/typing.html#typing.TYPE_CHECKING )

Example

from typing import TYPE_CHECKING
if TYPE_CHECKING:
    # coverage.py thinks this is not covered because TYPE_CHECKING is True at runtime
    ...
else:
    ....

You have to add # pragma: no_cover to the if block here because coverage.py assumes TYPE_CHECKING to be False, and therefore assumes one of the blocks does not have a test case. It would be good if you didn't have to, and blocks requiring TYPE_CHECKING to be True are assumed to not need test cases.

@retnikt retnikt added the bug Something isn't working label Aug 2, 2019
@nedbat
Copy link
Owner

nedbat commented Aug 2, 2019

You can solve this yourself today by adding this to your .coveragerc file:

[report]
exclude_lines = 
    pragma: no cover
    if TYPE_CHECKING:

More details about this setting are here: Advanced exclusion.

Since this is something that is configurable by the user, I would rather not change coverage.py to deal with it.

@nedbat nedbat closed this as completed Aug 2, 2019
lyz-code added a commit to lyz-code/blue-book that referenced this issue Oct 28, 2021
…e from urxvt to kitty

* It doesn't fuck up your terminal colors.
* You can use [peek](peek.md) to record your screen.
* Easier to extend.

feat(pytest#Excluding code from coverage): Exclude the `if TYPE_CHECKING` code from the coverage

If you want [other code to be
excluded](nedbat/coveragepy#831), for example the
statements inside the `if TYPE_CHECKING:` add to your `pyproject.toml`:

```toml
[tool.coverage.report]
exclude_lines = [
    # Have to re-enable the standard pragma
    'pragma: no cover',

    # Type checking can not be tested
    'if TYPE_CHECKING:',
]
```

fix(peek): Add note that it works with kitty

feat(prompt_toolkit_fullscreen_applications#Pass more than one key): Pass more than one key

To map an action to two key presses use `kb.add('g', 'g')`.

feat(prompt_toolkit_fullscreen_applications#styles): Add note on how to debug the styles of the components

Set the style to `bg:#dc322f` and it will be highlighted in red.
@cjw296
Copy link

cjw296 commented Jan 24, 2022

@nedbat - when would you expect code inside if TYPE_CHECKING blocks to be executed while code coverage measurement is in effect? How would you feel about making this part of the default exclude_lines?

@nedbat
Copy link
Owner

nedbat commented Jan 24, 2022

A few years later, I think you are right that it should be part of the default. If the user wants to measure those lines, they can.

@ikonst
Copy link

ikonst commented Apr 6, 2022

Can we also add ... to the defaults while we're at it?

@cjw296
Copy link

cjw296 commented Apr 7, 2022

@ikonst - I'm not 100% sure that's a good idea, but also as I've painfully discovered, you really mean \.\.\..

@ikonst
Copy link

ikonst commented Apr 7, 2022

Yeah, obviously in its escaped form. I think the sharp change in coverage would tell you :)

@cjw296
Copy link

cjw296 commented Apr 7, 2022

It can be quite the footgun when your projects are starting from 100% line coverage...

aucampia added a commit to aucampia/rdflib that referenced this issue Apr 19, 2022
This is so we can get a more accurate picture of coverage.

For more info see:
- nedbat/coveragepy#831
aucampia added a commit to RDFLib/rdflib that referenced this issue Apr 20, 2022
This is so we can get a more accurate picture of coverage.

For more info see:
- nedbat/coveragepy#831
stygian-coffee pushed a commit to dongit-org/python-reporter that referenced this issue Jul 29, 2022
@Kludex
Copy link
Contributor

Kludex commented Nov 9, 2022

A few years later, I think you are right that it should be part of the default. If the user wants to measure those lines, they can.

@nedbat Is the else from a if not TYPE_CHECKING: supposed to be supported as well? If yes, any tips where I can implement this part?

marcospri added a commit to hypothesis/lms that referenced this issue May 16, 2024
This might become the default in the future, see:

- nedbat/coveragepy#831
marcospri added a commit to hypothesis/lms that referenced this issue May 16, 2024
This might become the default in the future, see:

- nedbat/coveragepy#831
marcospri added a commit to hypothesis/lms that referenced this issue May 16, 2024
This might become the default in the future, see:

- nedbat/coveragepy#831
marcospri added a commit to hypothesis/lms that referenced this issue May 16, 2024
This might become the default in the future, see:

- nedbat/coveragepy#831
marcospri added a commit to hypothesis/lms that referenced this issue May 16, 2024
This might become the default in the future, see:

- nedbat/coveragepy#831
marcospri added a commit to hypothesis/lms that referenced this issue May 17, 2024
This might become the default in the future, see:

- nedbat/coveragepy#831
marcospri added a commit to hypothesis/lms that referenced this issue May 17, 2024
This might become the default in the future, see:

- nedbat/coveragepy#831
marcospri added a commit to hypothesis/lms that referenced this issue May 20, 2024
This might become the default in the future, see:

- nedbat/coveragepy#831
marcospri added a commit to hypothesis/lms that referenced this issue May 21, 2024
This might become the default in the future, see:

- nedbat/coveragepy#831
@ssbarnea
Copy link

ssbarnea commented Jun 3, 2024

While coveragepy itself seems to recognized the excluded lines as covered, it seems that uploading the reports to codecov.io does show them as not covered in their reporting. If someone spotted a workaround for that, please let me know.

@ikonst
Copy link

ikonst commented Jun 3, 2024

I think "excluded" is its own status in coveragepy, are you sure it's "covered" and not "excluded"? Or are you just concerned about coverage %?

@benjaoming
Copy link

benjaoming commented Jan 28, 2025

Am I correct that solving this happens in these lines:

# The default line exclusion regexes.
DEFAULT_EXCLUDE = [
r"#\s*(pragma|PRAGMA)[:\s]?\s*(no|NO)\s*(cover|COVER)",
]

...+ adding test case(s)?

@nedbat
Copy link
Owner

nedbat commented Jan 28, 2025

Yes, that is where the default exclusions are defined.

github-merge-queue bot pushed a commit to PennyLaneAI/pennylane that referenced this issue Apr 16, 2025
**Context:** 🤔 

The current dependency structure within the codebase is complex and
difficult to manage effectively. This PR introduces architectural
layering using `tach` to enforce clearer dependency boundaries between
different parts of PennyLane.

We are adopting a standard four-layer architecture: "ui", "tertiary",
"auxiliary", and "core",

- **ui**: This should NOT be in our source code and instead is a
user-interface layer. Examples of using this layer are `from pennylane
import *` or `import pennylane as qml`.
- **tertiary**: These modules can depend on *both* "auxiliary" and
"core" and its itself.
- **auxiliary**: These modules can depend on *only* "core" and itself.
- **core**: These modules can not depend on anything outside this layer.

This is the first step to untangling PennyLane source code.

**Description of the Change:** 📝 

* Updates `tach` to latest supported version.
* **`tach.toml` Configuration:** Updated `tach.toml` to define all
`pennylane` modules. Modules clearly fitting the new architecture were
assigned to their respective layers ("ui", "tertiary", "auxiliary",
"core"). Modules needing further analysis remain unlayered for future
PRs.
* Changed `QuantumFunctionError` in `math` module to `ValueError`.

❗ `labs` and `ftqc` are still considered "closed" modules (nothing can
import from them). ❗

* Updates `.coveragerc` to ignore `TYPE_CHECKING` conditionals
(nedbat/coveragepy#831).

* **Import Optimization:** Reviewed and updated import statements in
relevant modules to import only strictly necessary components,
minimizing dependencies without code refactoring.
* **Refactoring:** Removed the dependency on
`pennylane.operation.Operator` from `pennylane/numpy/tensor.py` . This
dependency contributed to circular import issues, and code coverage
indicated the relevant logic was untested and likely unused, justifying
its removal.



**Benefits:** ❤️ 

* Enforces dependency rules via `tach` for all modules assigned to a
layer, preventing invalid imports and improving architectural integrity.
* Provides a clearer, more organized structure for dependencies,
enhancing code maintainability.
* Establishes a foundation to prevent problematic dependency cycles and
invalid layer traversals going forward.

**Possible Drawbacks:** ⚠️ 

As a direct result of enforcing these architectural layers there are few
issues I can forsee,

- modules assigned to specific layers can no longer use the top-level
`import pennylane as qml` pattern. Imports must target specific
submodules according to the allowed dependencies defined in `tach.toml`.
This enforces the intended layering and prevents unintended coupling.
- a developer might hit a situation where they *need* to do something
that involves breaking this dependency graph. In this case, they should
consider re-factoring or just temporarily disable this dependency
requirement in the `tach.toml`.

[sc-88205]

---------

Co-authored-by: Astral Cai <[email protected]>
Co-authored-by: Christina Lee <[email protected]>
austingmhuang pushed a commit to PennyLaneAI/pennylane that referenced this issue May 6, 2025
**Context:** 🤔 

The current dependency structure within the codebase is complex and
difficult to manage effectively. This PR introduces architectural
layering using `tach` to enforce clearer dependency boundaries between
different parts of PennyLane.

We are adopting a standard four-layer architecture: "ui", "tertiary",
"auxiliary", and "core",

- **ui**: This should NOT be in our source code and instead is a
user-interface layer. Examples of using this layer are `from pennylane
import *` or `import pennylane as qml`.
- **tertiary**: These modules can depend on *both* "auxiliary" and
"core" and its itself.
- **auxiliary**: These modules can depend on *only* "core" and itself.
- **core**: These modules can not depend on anything outside this layer.

This is the first step to untangling PennyLane source code.

**Description of the Change:** 📝 

* Updates `tach` to latest supported version.
* **`tach.toml` Configuration:** Updated `tach.toml` to define all
`pennylane` modules. Modules clearly fitting the new architecture were
assigned to their respective layers ("ui", "tertiary", "auxiliary",
"core"). Modules needing further analysis remain unlayered for future
PRs.
* Changed `QuantumFunctionError` in `math` module to `ValueError`.

❗ `labs` and `ftqc` are still considered "closed" modules (nothing can
import from them). ❗

* Updates `.coveragerc` to ignore `TYPE_CHECKING` conditionals
(nedbat/coveragepy#831).

* **Import Optimization:** Reviewed and updated import statements in
relevant modules to import only strictly necessary components,
minimizing dependencies without code refactoring.
* **Refactoring:** Removed the dependency on
`pennylane.operation.Operator` from `pennylane/numpy/tensor.py` . This
dependency contributed to circular import issues, and code coverage
indicated the relevant logic was untested and likely unused, justifying
its removal.



**Benefits:** ❤️ 

* Enforces dependency rules via `tach` for all modules assigned to a
layer, preventing invalid imports and improving architectural integrity.
* Provides a clearer, more organized structure for dependencies,
enhancing code maintainability.
* Establishes a foundation to prevent problematic dependency cycles and
invalid layer traversals going forward.

**Possible Drawbacks:** ⚠️ 

As a direct result of enforcing these architectural layers there are few
issues I can forsee,

- modules assigned to specific layers can no longer use the top-level
`import pennylane as qml` pattern. Imports must target specific
submodules according to the allowed dependencies defined in `tach.toml`.
This enforces the intended layering and prevents unintended coupling.
- a developer might hit a situation where they *need* to do something
that involves breaking this dependency graph. In this case, they should
consider re-factoring or just temporarily disable this dependency
requirement in the `tach.toml`.

[sc-88205]

---------

Co-authored-by: Astral Cai <[email protected]>
Co-authored-by: Christina Lee <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

7 participants