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

Fix package CI for pysam errors. #13644

Merged
merged 1 commit into from
Mar 31, 2022
Merged

Conversation

jmchilton
Copy link
Member

@jmchilton jmchilton commented Mar 30, 2022

Fix CircleCI for now? Pin pysam since 0.19.0 seems to have incomplete type definitions.

Failed build example: https://app.circleci.com/pipelines/github/galaxyproject/galaxy/22088/workflows/f9d6b681-da6d-4e0e-9867-baeb1fc73968/jobs/167487

How to test the changes?

(Select all options that apply)

  • This is a refactoring of components with existing test coverage.

License

  • I agree to license these contributions under Galaxy's current license.
  • I agree to allow the Galaxy committers to license these and all my past contributions to the core galaxy codebase under the MIT license. If this condition is an issue, uncheck and just let us know why with an e-mail to [email protected].

@nsoranzo
Copy link
Member

I hoped you knew some trick to make pysam's type annotations work 😢 I had a look and it seems to be there in the .pyi files, not sure why it's not picked up by mypy.

Maybe @mr-c has an idea?

@mvdbeek mvdbeek merged commit 2b44828 into galaxyproject:dev Mar 31, 2022
@mr-c
Copy link
Contributor

mr-c commented Mar 31, 2022

I hoped you knew some trick to make pysam's type annotations work 😢 I had a look and it seems to be there in the .pyi files, not sure why it's not picked up by mypy.

Maybe @mr-c has an idea?

Got a link to the error message?

@nsoranzo nsoranzo deleted the pin_pysam branch March 31, 2022 09:16
@nsoranzo
Copy link
Member

@mr-c It's in the issue description.

@mr-c
Copy link
Contributor

mr-c commented Mar 31, 2022

Possibly due to use of star imports in pysam/__init__.py but with a short __all__ list of exports

pysam.tabix_compress(input_fname, output_fname, force=True)

There is no tabix_compress in https://github.com/pysam-developers/pysam/blob/bc1e4de622a03fe8dd2c6731de6a53f0e2c98f00/pysam/__init__.py#L35

But there is a tabix_compress in https://github.com/pysam-developers/pysam/blob/2a5e55e85b4c8013e6140693175e8f7cb20a4603/pysam/libctabix.pyi#L73

So either pysam needs to fill out __all__ in https://github.com/pysam-developers/pysam/blob/2a5e55e85b4c8013e6140693175e8f7cb20a4603/pysam/libctabix.pyi or galaxy uses things like pysam.libctabix.tabix_compress

@nsoranzo
Copy link
Member

@mr-c Thanks for investigating! I had a look at your option 2 yesterday, but I have a feeling that wouldn't work:

$ virtualenv pysam_0.19.0_venv
created virtual environment CPython3.8.10.final.0-64 in 83ms
...
$ . pysam_0.19.0_venv/bin/activate
(pysam_0.19.0_venv) $ pip install pysam
Collecting pysam
  Using cached pysam-0.19.0-cp38-cp38-manylinux_2_12_x86_64.manylinux2010_x86_64.whl (16.3 MB)
Installing collected packages: pysam
Successfully installed pysam-0.19.0
(pysam_0.19.0_venv) $ python
Python 3.8.10 (default, Mar 15 2022, 12:22:08) 
[GCC 9.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from typing import get_type_hints
>>> from pysam.libctabix import tabix_compress
>>> get_type_hints(tabix_compress)
{}
>>> tabix_compress.__annotations__
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'builtin_function_or_method' object has no attribute '__annotations__'

@mr-c
Copy link
Contributor

mr-c commented Mar 31, 2022

I don't think that get_type_hints pulls from .pyi files, so that might not be a fair test.

https://bugs.python.org/issue34700

python/mypy#1934

https://stackoverflow.com/questions/45829894/how-to-get-type-annotation-from-stub-files-pyi-at-runtime-in-python

@nsoranzo
Copy link
Member

I don't think that get_type_hints pulls from .pyi files, so that might not be a fair test.

Oh, you're right, importing from the submodules seems to work! Thanks a bunch!!

@nsoranzo
Copy link
Member

Interestingly, these errors seem to go away when upgrading from mypy 0.910 (version we currently pin) to >= 0.940 . Unfortunately, the new version also finds 13 new errors, so we'd need to take a look at those, but probably that's still the better option.

@mr-c
Copy link
Contributor

mr-c commented Mar 31, 2022

Heh. Progress!

@jmchilton
Copy link
Member Author

@nsoranzo are you working through that upgrade or would like me to? I always assumed this was a short term stop gap.

@nsoranzo
Copy link
Member

@jmchilton I'm on it, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants