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

python 3.5 support #57

Closed
jeff-regier opened this issue Jun 13, 2018 · 17 comments
Closed

python 3.5 support #57

jeff-regier opened this issue Jun 13, 2018 · 17 comments
Labels

Comments

@jeff-regier
Copy link

I'm getting the error below when I try to install loompy on Python 3.5, even though everything works with Python 3.6.

    Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File "/tmp/pip-install-t8nyqf3g/loompy/setup.py", line 26
        download_url=f"https://github.com/linnarsson-lab/loompy/archive/{__version__}.tar.gz",
                                                                                            ^
    SyntaxError: invalid syntax

I guess the f"..." notation is new in 3.6:

jeff@dean ~/git/scVI $ python3.6
Python 3.6.5 |Anaconda, Inc.| (default, Apr 26 2018, 13:46:40) 
[GCC 7.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> a = "hi"
>>> f"{a}"
'hi'
>>> 
jeff@dean ~/git/scVI $ python3.5
Python 3.5.2 (default, Nov 23 2017, 16:37:01) 
[GCC 5.4.0 20160609] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> a = "hi"
>>> f"{a}"
  File "<stdin>", line 1
    f"{a}"
         ^
SyntaxError: invalid syntax

Would you consider adding support for Python 3.5 to loompy?

@slinnarsson
Copy link
Contributor

String interpolation in Python 3.6 is a great thing, and makes the code so much easier to read!

But, on the other hand, I want loompy to be widely used. Do you have a compelling reason why you need to stay with Python 3.5?

A quick search indicates that we have ~90 instances of using string interpolation. It would not be a huge amount of work to revert those to the old syntax, but there's always the risk of introducing a bug or two.

@jeff-regier
Copy link
Author

I always use Python 3.6 so not an issue for me; but we want scVI to be widely used and many people aren't using the latest Python release. We just started using tox to test scVI and all its dependencies against other versions of Python. Currently loompy is our only dependency that isn't Python 3.5 compatible. Most libraries support both Python 2 (though the future package) and Python 3.

That said, I agree that string interpolation is a great thing, and 90 instances is a lot. So I don't know whether its worth it just for scVI -- we'll keep using loompy regardless. I mainly posted the issue because I thought it might be an oversight.

@slinnarsson
Copy link
Contributor

Ok got it. Feel free to send a pull request ;-) For now, I’ll consider it a feature request for loompy3.

@slinnarsson
Copy link
Contributor

I realized that we also use Python 3.6 variable type hints, which is going to be a pain to find and remove (or is there an automated tool for this?).

@rcurrie
Copy link

rcurrie commented Jul 1, 2018

I'm trying to use loompy with tensorflow on single cell data. At the moment the tensorflow community is primarily on python 3.5 in part due to the complex interactions with GPU drivers. I suspect many are looking at machine learning on single cell and as such this particular combination may be important. Will see if I can find a solution and report back.

@Xparx
Copy link

Xparx commented Aug 30, 2018

Hi,
I just ran in to this issue. One argument for supporting python 3.5 on top of just broadening people who can use the tool, is that it is the default version for e.g. ubuntu16.04. I quick google seems to suggest that it is the version for stable debian as well https://packages.debian.org/search?keywords=python3.

I'm going to see if I can get python >3.6 up and running.

@Xparx
Copy link

Xparx commented Aug 30, 2018

You could also add a
python_requires='>=3.6',
in the setup.py file which would probably simplify life for people running in to this issue.

@slinnarsson
Copy link
Contributor

Good idea. I will add it. To get started quickly, I recommend Anaconda, which installs in your home directory so it doesn’t mess with your system’s python.

@slinnarsson
Copy link
Contributor

I found py-backwards and it can indeed be used to backport loompy to at least Python 3.5. The following works, and creates a Python 3.5-compatible loompy:

pip install py-backwards
cd ~/code  (the directory where loompy resides)
cp -R loompy loompy-3.5  # Make a copy of the repo
cd loompy-3.5/loompy
for f in *.py; do py-backwards -i $f -o $f -t 3.5; done  # backport to Python 3.5
cd ..
python setup.py install

There is also py-backwards-packager, but it doesn't backport source distributions (sdist), only binary (i.e. bdist). Loompy is packaged as a source distribution so that we can support any platform.

If you know a way to create a bdist package that works on any platform, the above could be a solution. It's also a viable manual workaround right now for anyone who needs Python <3.6.

@slinnarsson
Copy link
Contributor

I updated the docs with instructions for installing loompy from source on Python <3.6

@slinnarsson
Copy link
Contributor

I'll close this now, but feel free to send a pull request if you figure out how to automate py-backwards as part of the packaging process.

@Xparx
Copy link

Xparx commented Sep 3, 2018

Sorry for the late reply. Somehow I didn't get a notification for your comments.

I noticed a couple of things when using py-backwards.

  1. You also need to run the py-backwards on the setup.py file
    py-backwards -i setup.py -o setup.py -t 3.5

  2. I also get a syntax error on the following line https://github.com/linnarsson-lab/loompy/blob/master/loompy/loom_repo_client.py#L99
    adding a comma on the end of the line fixes it for me and py-backwards will not work without it.
    After doing this I can install-ish.

  3. However one error seems to remain after i do py-backwards and it breaks reading from loom files,
    The following happens when trying to do py-backwards on loom_validator.py

Transformation error in "loom_validator.py", transformer "FormattedValuesTransformer" failed with:
Traceback (most recent call last):
  File "/home/andreas/.virtualenvs/deep_learning_playground/lib/python3.5/site-packages/py_backwards/compiler.py", line 31, in _transform
    result = transformer.transform(working_tree)
  File "/home/andreas/.virtualenvs/deep_learning_playground/lib/python3.5/site-packages/py_backwards/transformers/base.py", line 28, in transform
    inst.visit(tree)
  File "/home/andreas/.virtualenvs/deep_learning_playground/lib/python3.5/site-packages/typed_ast/ast3.py", line 275, in visit
    return visitor(node)
  File "/home/andreas/.virtualenvs/deep_learning_playground/lib/python3.5/site-packages/typed_ast/ast3.py", line 330, in generic_visit
    value = self.visit(value)
  File "/home/andreas/.virtualenvs/deep_learning_playground/lib/python3.5/site-packages/typed_ast/ast3.py", line 275, in visit
    return visitor(node)
  File "/home/andreas/.virtualenvs/deep_learning_playground/lib/python3.5/site-packages/typed_ast/ast3.py", line 330, in generic_visit
    value = self.visit(value)
  File "/home/andreas/.virtualenvs/deep_learning_playground/lib/python3.5/site-packages/typed_ast/ast3.py", line 275, in visit
    return visitor(node)
  File "/home/andreas/.virtualenvs/deep_learning_playground/lib/python3.5/site-packages/typed_ast/ast3.py", line 330, in generic_visit
    value = self.visit(value)
  File "/home/andreas/.virtualenvs/deep_learning_playground/lib/python3.5/site-packages/typed_ast/ast3.py", line 275, in visit
    return visitor(node)
  File "/home/andreas/.virtualenvs/deep_learning_playground/lib/python3.5/site-packages/typed_ast/ast3.py", line 330, in generic_visit
    value = self.visit(value)
  File "/home/andreas/.virtualenvs/deep_learning_playground/lib/python3.5/site-packages/typed_ast/ast3.py", line 275, in visit
    return visitor(node)
  File "/home/andreas/.virtualenvs/deep_learning_playground/lib/python3.5/site-packages/typed_ast/ast3.py", line 330, in generic_visit
    value = self.visit(value)
  File "/home/andreas/.virtualenvs/deep_learning_playground/lib/python3.5/site-packages/typed_ast/ast3.py", line 275, in visit
    return visitor(node)
  File "/home/andreas/.virtualenvs/deep_learning_playground/lib/python3.5/site-packages/typed_ast/ast3.py", line 339, in generic_visit
    new_node = self.visit(old_value)
  File "/home/andreas/.virtualenvs/deep_learning_playground/lib/python3.5/site-packages/typed_ast/ast3.py", line 275, in visit
    return visitor(node)
  File "/home/andreas/.virtualenvs/deep_learning_playground/lib/python3.5/site-packages/typed_ast/ast3.py", line 330, in generic_visit
    value = self.visit(value)
  File "/home/andreas/.virtualenvs/deep_learning_playground/lib/python3.5/site-packages/typed_ast/ast3.py", line 275, in visit
    return visitor(node)
  File "/home/andreas/.virtualenvs/deep_learning_playground/lib/python3.5/site-packages/py_backwards/transformers/formatted_values.py", line 36, in visit_JoinedStr
    return self.generic_visit(join_call)  # type: ignore
  File "/home/andreas/.virtualenvs/deep_learning_playground/lib/python3.5/site-packages/typed_ast/ast3.py", line 330, in generic_visit
    value = self.visit(value)
  File "/home/andreas/.virtualenvs/deep_learning_playground/lib/python3.5/site-packages/typed_ast/ast3.py", line 275, in visit
    return visitor(node)
  File "/home/andreas/.virtualenvs/deep_learning_playground/lib/python3.5/site-packages/typed_ast/ast3.py", line 330, in generic_visit
    value = self.visit(value)
  File "/home/andreas/.virtualenvs/deep_learning_playground/lib/python3.5/site-packages/typed_ast/ast3.py", line 275, in visit
    return visitor(node)
  File "/home/andreas/.virtualenvs/deep_learning_playground/lib/python3.5/site-packages/py_backwards/transformers/formatted_values.py", line 19, in visit_FormattedValue
    template = ''.join(['{:', node.format_spec.s, '}'])  # type: ignore
AttributeError: 'JoinedStr' object has no attribute 's'

If I still install loompy and try to read som data with scanpy.

import scanpy.api as sc

In [3]: data = sc.read_loom('./datafile.loom.gz')
/home/andreas/.virtualenvs/deep_learning_playground/lib/python3.5/importlib/_bootstrap.py:222: RuntimeWarning: numpy.dtype size changed, may indicate binary incompatibility. Expected 96, got 88
  return f(*args, **kwds)
Traceback (most recent call last):

  File "/home/andreas/.virtualenvs/deep_learning_playground/lib/python3.5/site-packages/IPython/core/interactiveshell.py", line 2961, in run_code
    exec(code_obj, self.user_global_ns, self.user_ns)

  File "<ipython-input-3-8353dae35485>", line 1, in <module>
    data = sc.read_loom('./all_sgete_4GU75.loom.gz')

  File "/home/andreas/.virtualenvs/deep_learning_playground/lib/python3.5/site-packages/anndata/readwrite/read.py", line 145, in read_loom
    from loompy import connect

  File "/home/andreas/.virtualenvs/deep_learning_playground/lib/python3.5/site-packages/loompy-2.0.13-py3.5.egg/loompy/__init__.py", line 12, in <module>
    from .loom_validator import LoomValidator

  File "/home/andreas/.virtualenvs/deep_learning_playground/lib/python3.5/site-packages/loompy-2.0.13-py3.5.egg/loompy/loom_validator.py", line 17
    self.errors: List[str] = []  #: Errors found during validation
               ^
SyntaxError: invalid syntax

Looks like an error in loom_validator.py but I can't figure out what it is. My syntax checker marks some things but that is simply becouse the file have failed to convert to 3.5, but I can't figure out why.

Some hack and slash later it seems to be an issue with py-backwards.
I chopped up the loom_validator function and ran py-backwards. It seems to trigger on this part/function;

def validate_spec(self, file: h5py.File) -> bool:
        """
        Validate the LoomConnection object against the format specification.

        Args:
                file:                   h5py File object

        Returns:
                True if the file conforms to the specs, else False

        Remarks:
                Upon return, the instance attributes 'self.errors' and 'self.warnings' contain
                lists of errors and warnings, and the 'self.summary' attribute contains a summary
                of the file contents.
        """
        matrix_types = ["float16", "float32", "float64", "int8", "int16", "int32", "int64", "uint8", "uint16", "uint32", "uint64"]
        vertex_types = ["int8", "int16", "int32", "int64", "uint8", "uint16", "uint32", "uint64"]
        weight_types = ["float16", "float32", "float64"]

        def delay_print(text: str) -> None:
                self.summary.append(text)

        def dt(t: str) -> str:
                if str(t).startswith("|S"):
                        return f"string"
                return str(t)

        width_ra = max([len(x) for x in (file["row_attrs"].keys())])
        width_ca = max([len(x) for x in (file["col_attrs"].keys())])
        width_globals = max([len(x) for x in file.attrs.keys()])
        width_layers = 0
        if "layers" in file and len(file["layers"]) > 0:
                width_layers = max([len(x) for x in file["layers"].keys()])
        width_layers = max(width_layers, len("Main matrix"))
        width = max(width_ca, width_ra, width_globals)

        delay_print("Global attributes:")
        for key, value in file.attrs.items():
                if type(value) is str:
                        self.warnings.append(f"Global attribute '{key}' has dtype string, which will be deprecated in future Loom versions")
                        delay_print(f"{key: >{width}} string")
                else:
                        delay_print(f"{key: >{width}} {dt(file.attrs[key].dtype)}")

        return len(self.errors) == 0

but I can't make sense of the py-backwards error.
I posted an issue here: nvbn/py-backwards#48

@slinnarsson
Copy link
Contributor

Hi

  1. Oh, right. I will add it to the docs.

  2. I removed LoomRepoClient, which was not supposed to go in the master branch. I'm experimenting and accidentally checked that one in. If you pull the latest revision, it will be gone.

  3. Sorry about that. I did test py-backwards on a previous commit (before adding the validator code). I will see if I can understand what triggers the issue. In the meantime, you can call loompy.connect(validate=False) to prevent the validator from running.

@slinnarsson
Copy link
Contributor

I guess it's the nested curly braces in the formatting specification, and I posted some ideas at nvbn/py-backwards#48.

You can also try using the commit just before I added the validator (git checkout 0f5c934679822c7babb020928516b9c7e01c3f7e)

@Xparx
Copy link

Xparx commented Sep 15, 2018

Just a heads up. After me and my laptop went through an existential crisis triggered by me wanting to update ubuntu instead of just simply python, I am finally back where I started but have installed python3.6.
Needless to say these issues above are not relevant to me anymore.

Please let me know if you need help testing something. I was planning to test your suggestion but will not do it unless it is needed.
Thanks for the help!

@gamazeps
Copy link

Ok got it. Feel free to send a pull request ;-) For now, I’ll consider it a feature request for loompy3.

Is the PR still ok ?
If so I'll send one in the week.

For the record: I am exactly in the situation of using loompy in scvi, and for some god awful reason I am stuck with python3.5.

@slinnarsson
Copy link
Contributor

Hi @gamazeps - sure, I'll be happy to have a look!

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

No branches or pull requests

5 participants