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

Andor CCD DU401 driver #32

Merged
merged 10 commits into from
Jan 22, 2020
Merged

Andor CCD DU401 driver #32

merged 10 commits into from
Jan 22, 2020

Conversation

lankes-fzj
Copy link
Contributor

New driver for 'Andor CCD DU401'

(based on SvenBo90/Qcodes@2cff137)

@astafan8
Copy link
Contributor

@lankes-fzj see commit e8344e0 about WinDLL type annotation and sys.platform usage. Otherwise type checking with mypy fails the CI.

@lankes-fzj
Copy link
Contributor Author

lankes-fzj commented Jan 20, 2020

Do you have any idea, why mypy fails with the error "Cannot determine type of 'dll'"? This didn't happen to PRs #28 and #29, although they are working the same way...

@jenshnielsen
Copy link
Collaborator

I am not sure but I suggest that you just put an assert sys.platform == 'win32' at the top of the file just after the imports as suggested here https://mypy.readthedocs.io/en/stable/common_issues.html#python-version-and-system-platform-checks

@lankes-fzj
Copy link
Contributor Author

I am not sure but I suggest that you just put an assert sys.platform == 'win32' at the top of the file just after the imports as suggested here https://mypy.readthedocs.io/en/stable/common_issues.html#python-version-and-system-platform-checks

But then the tests would fail on linux, wouldn't they?

@jenshnielsen
Copy link
Collaborator

Which tests? As documented in the link above this will make mypy stop doing any cheking of that file on platforms that are not windows. As far as I can see there are no other tests for this at the moment

@jenshnielsen
Copy link
Collaborator

jenshnielsen commented Jan 22, 2020

Sorry that fails the docs build as it cannot import the code :(

The best workaround that I can come up with is to explicitly annotate self.dll as Any on Windows platforms.

I think the following will work (even if its a bit strange)

from typing import Dict, List, Optional, Tuple, Any

...

    def __init__(self, dll_path: Optional[str] = None, verbose: bool = False):
        if sys.platform != 'win32':
            self.dll: Any = None
            raise OSError("\"atmcd64d\" is only compatible with Microsoft Windows")
        else:
            self.verbose = verbose
            self.dll = ctypes.windll.LoadLibrary(dll_path or self._dll_path)

Edited to change from cdll to None with Any type

@lankes-fzj
Copy link
Contributor Author

@jenshnielsen Thank you very much for your help :)

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