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

Hotfixes #70

Merged
merged 20 commits into from
Feb 7, 2024
Merged

Hotfixes #70

merged 20 commits into from
Feb 7, 2024

Conversation

THuckemann
Copy link
Collaborator

Just some minor import related fixes to make everything work

@THuckemann THuckemann self-assigned this Jan 19, 2024
Copy link

github-actions bot commented Jan 19, 2024

Test Results

 3 files  ±0   3 suites  ±0   21s ⏱️ +12s
24 tests ±0  24 ✅ ±0  0 💤 ±0  0 ❌ ±0 
72 runs  ±0  72 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 8b86fe7. ± Comparison against base commit 16e72dc.

♻️ This comment has been updated with latest results.

@THuckemann
Copy link
Collaborator Author

Not sure why the Unit Test fails. I had to add the undescore in front of _catch_interrupts as this was apparently changed in QCodes some time ago (could not find it in the changelog though), it works fine on all setups I use. Maybe a different Qcodes Version is used for the tests?

@newton-per-sqm newton-per-sqm self-assigned this Feb 7, 2024
@newton-per-sqm
Copy link
Collaborator

The unit-tests say:

ERROR src/tests/instrument_test.py - TypeError: getfixtureclosure() got an unexpected keyword argument 'initialnames'
ERROR src/tests/mapping_test.py
ERROR src/tests/metadata_test.py - TypeError: getfixtureclosure() got an unexpected keyword argument 'initialnames'

@newton-per-sqm
Copy link
Collaborator

newton-per-sqm commented Feb 7, 2024

Conceptionally, such CI flows are always build with new environment setups. Your pytest environment is using qcodes >= 0.40, so always the newest published version:

Collecting qcodes>=0.40.0 (from QuMADA==0.4.2.post82+ge9eae12)
  Downloading qcodes-0.44.0-py3-none-any.whl.metadata (9.2 kB)

@newton-per-sqm
Copy link
Collaborator

@THuckemann: Could you check the files mentioned in my previous comment (#70 (comment)) wether they are working on your setup?

@THuckemann
Copy link
Collaborator Author

@THuckemann: Could you check the files mentioned in my previous comment (#70 (comment)) wether they are working on your setup?

They all work (after installing pytest and related modules).
Also I would put more focus on this part of the test report, as renaming catch_interrupts to _catch_interrupts was one of the changes in the last commit. I could try to do the import with a try/except to cover the old import without an underscore to pinpoint the problem.

ImportError while importing test module '/home/runner/work/QuMADA/QuMADA/src/tests/mapping_test.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
src/tests/mapping_test.py:50: in
from qumada.measurement.scripts.generic_measurement import Generic_1D_Sweep
src/qumada/measurement/scripts/init.py:21: in
from .generic_measurement import (
src/qumada/measurement/scripts/generic_measurement.py:33: in
from qumada.measurement.doNd_enhanced.doNd_enhanced import (
src/qumada/measurement/doNd_enhanced/doNd_enhanced.py:43: in
from qcodes.dataset.dond.do_nd_utils import (
E ImportError: cannot import name '_catch_interrupts' from 'qcodes.dataset.dond.do_nd_utils' (/opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/site-packages/qcodes/dataset/dond/do_nd_utils.py)

@THuckemann
Copy link
Collaborator Author

Ok, this didnt solve the problem, although the import related part disappeared from the log.
I presume this is just an issue with pytest 8.0.0 then (see here).
Can we downgrade to the previous version until this is resolved?

Copy link

codecov bot commented Feb 7, 2024

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (16e72dc) 29.45% compared to head (8b86fe7) 29.53%.

Files Patch % Lines
.../qumada/measurement/doNd_enhanced/doNd_enhanced.py 71.42% 2 Missing ⚠️
src/qumada/measurement/scripts/__init__.py 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #70      +/-   ##
==========================================
+ Coverage   29.45%   29.53%   +0.08%     
==========================================
  Files          35       35              
  Lines        3368     3372       +4     
==========================================
+ Hits          992      996       +4     
  Misses       2376     2376              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@THuckemann
Copy link
Collaborator Author

Ok, apparently this was really the issue. The pytest version is now fixed to 7.4.4, once the issue appearing with 8.0.0 is fixed we might want allow updates again.

THuckemann and others added 2 commits February 7, 2024 13:10
@THuckemann
Copy link
Collaborator Author

The import issue still remains, but at least we can bypass it with the current try except block. I will commit the merge now as the hotfixes are required to make the current version work.

@THuckemann THuckemann merged commit c269d75 into main Feb 7, 2024
8 checks passed
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.

2 participants