Skip to content

Conversation

@nquetschlich
Copy link
Collaborator

I still get an error for the mqt.ddsim import and do not really understand why it is there, since mqt.ddsim is both listed as an additional dependency in the pre-commit-config.yaml and also excluded in the pyproject.toml. I would have assumed, that either of those two options should resolve the problem:

error: Module "mqt" has no attribute "ddsim" 
[attr-defined]
    from mqt import ddsim

@codecov
Copy link

codecov bot commented Feb 10, 2023

Codecov Report

Merging #16 (85994e7) into main (923bcb3) will increase coverage by 1.1%.
The diff coverage is 100.0%.

@@           Coverage Diff           @@
##            main     #16     +/-   ##
=======================================
+ Coverage   91.6%   92.8%   +1.1%     
=======================================
  Files          3       3             
  Lines         24      28      +4     
=======================================
+ Hits          22      26      +4     
  Misses         2       2             
Impacted Files Coverage Δ
tests/test_csp.py 100.0% <100.0%> (ø)
tests/test_tsp.py 100.0% <100.0%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@burgholzer
Copy link
Member

Hm. I can have a look at that later. Could you, first, merge #15 and rebase this PR on it, so that this PR really only contains changes related to the PR? Makes it easier to check.

A few things as a heads-up. DDSIM is not yet typed. So it definitely needs to be excluded somehow.
The reason why your exclude is not working is because the error already happens at the mqt module scope and not at mqt.ddsim.*. However, there should be a solution for that.

@burgholzer
Copy link
Member

You will also have to move the mypy check to a separate CI workflow similar to how this was handled in the predictor.
This is due to qiskit-terra (and it's dependencies) exceeding the memory limits of the pre-commit.ci service.

@nquetschlich
Copy link
Collaborator Author

Hm. I can have a look at that later. Could you, first, merge #15 and rebase this PR on it, so that this PR really only contains changes related to the PR? Makes it easier to check.

A few things as a heads-up. DDSIM is not yet typed. So it definitely needs to be excluded somehow. The reason why your exclude is not working is because the error already happens at the mqt module scope and not at mqt.ddsim.*. However, there should be a solution for that.

Done. Also added the CI workflow.

@burgholzer
Copy link
Member

Hm. I can have a look at that later. Could you, first, merge #15 and rebase this PR on it, so that this PR really only contains changes related to the PR? Makes it easier to check.

A few things as a heads-up. DDSIM is not yet typed. So it definitely needs to be excluded somehow. The reason why your exclude is not working is because the error already happens at the mqt module scope and not at mqt.ddsim.*. However, there should be a solution for that.

Done. Also added the CI workflow.

You still need to skip the mypy check in the ci (see the pre-commit Config of the predictor). Otherwise it is run twice.
You can also add back the mqt.ddsim package then.

@nquetschlich
Copy link
Collaborator Author

Hm. I can have a look at that later. Could you, first, merge #15 and rebase this PR on it, so that this PR really only contains changes related to the PR? Makes it easier to check.

A few things as a heads-up. DDSIM is not yet typed. So it definitely needs to be excluded somehow. The reason why your exclude is not working is because the error already happens at the mqt module scope and not at mqt.ddsim.*. However, there should be a solution for that.

Done. Also added the CI workflow.

You still need to skip the mypy check in the ci (see the pre-commit Config of the predictor). Otherwise it is run twice. You can also add back the mqt.ddsim package then.

Oh, my bad. I fixed both issues.

Copy link
Member

@burgholzer burgholzer left a comment

Choose a reason for hiding this comment

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

Many thanks for adding mypy here. I just have a couple of really small comments.
Feel free to ignore them and merge as is.

Getting mypy to stop complaining was easier than I thought originally. I just replaced
from mqt import dddim with from mqt.ddsim import <...>. That way the configuration from the pyproject.toml takes effect.
I suspect the original error is due to DDSIM not yet being typed and thus not getting installed as part of the mypy check.

@nquetschlich nquetschlich merged commit a723c9a into main Feb 10, 2023
@nquetschlich nquetschlich deleted the mypy branch February 10, 2023 14:12
nquetschlich added a commit that referenced this pull request Feb 10, 2023
I still get an error for the `mqt.ddsim` import and do not really
understand why it is there, since `mqt.ddsim` is both listed as an
additional dependency in the `pre-commit-config.yaml` and also excluded
in the `pyproject.toml`. I would have assumed, that either of those two
options should resolve the problem:

```
error: Module "mqt" has no attribute "ddsim" 
[attr-defined]
    from mqt import ddsim
```

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Lukas Burgholzer <lukas.burgholzer@jku.at>
tobi-forster pushed a commit that referenced this pull request May 15, 2025
…/setup-python-5

Bump actions/setup-python from 4 to 5
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.

3 participants