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 installation #25

Merged
merged 1 commit into from
May 20, 2021
Merged

fix installation #25

merged 1 commit into from
May 20, 2021

Conversation

pjb7687
Copy link
Contributor

@pjb7687 pjb7687 commented May 18, 2021

No description provided.

@ilia-kats
Copy link
Collaborator

pip install git+https://github.com/gtca/muon works for me. What exactly is the problem that you're having?

@pjb7687
Copy link
Contributor Author

pjb7687 commented May 18, 2021

It didn't work when I did:

python setup.py install

@pjb7687
Copy link
Contributor Author

pjb7687 commented May 18, 2021

And FYI the problem is not the installation itself; it worked without error. But the _core module was not available after installation when I tried to import muon.

@ilia-kats
Copy link
Collaborator

Did you really mean to write python install setup.py? Or python setup.py install? I can import muon after installation just fine. Which Python version and setuptools version do you have?

@pjb7687
Copy link
Contributor Author

pjb7687 commented May 18, 2021

Did you really mean to write python install setup.py? Or python setup.py install? I can import muon after installation just fine. Which Python version and setuptools version do you have?

Sorry, the latter.

❯ python
Python 3.9.4 | packaged by conda-forge | (default, May 10 2021, 22:13:33)
[GCC 9.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import setuptools
>>> setuptools.__version__
'52.0.0.post20210125'
>>> import muon
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/parkj/miniconda3/envs/muon/lib/python3.9/site-packages/muon-0.1.0-py3.9.egg/muon/__init__.py", line 1, in <module>
    from ._core.mudata import MuData
ModuleNotFoundError: No module named 'muon._core'
>>>

@ilia-kats
Copy link
Collaborator

OK, I have setuptools 56.1.0. I couldn't find anything in the changelogs, but it looks like find_packages now also finds PEP420-style namespace packages. @gtca I don't think _core and _rna are intended to be namespace packages, this PR looks good to me.

@pjb7687
Copy link
Contributor Author

pjb7687 commented May 18, 2021

pip install git+https://github.com/gtca/muon works for me. What exactly is the problem that you're having?

Just confirmed it works fine. This only happens when muon was installed with python setup.py install.

@ilia-kats
Copy link
Collaborator

Ah sorry, just now had a look at pyproject.toml. When you're installing with pip, setuptools is not used at all, but poetry is. @gtca does it even make sense to have setup.py at all anymore? It's not required when using recent versions of pip, pyproject.toml is sufficient, and we would only have one build system to support.

@gtca
Copy link
Collaborator

gtca commented May 19, 2021

Thanks, I'll merge it, and I'd keep setup.py as a familiar way to install it, at least for now.

While we're all here, I have to say I wasn't able to reproduce the issue above. That's true for a few systems where I am using muon, this is just one example:

❯ python3 setup.py install
❯ python3
Python 3.8.6 (default, Nov 20 2020, 18:29:40)
[Clang 12.0.0 (clang-1200.0.32.27)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import setuptools
>>> setuptools.__version__
'50.3.2'
>>> import muon
>>> from importlib import metadata
>>> metadata.version(muon)
'0.1.0'

@ilia-kats
Copy link
Collaborator

Apparently setup.py install hasn't been supported by upstream for a while and will be removed in the future: pypa/setuptools#510 (comment) . Not sure if keeping it around here is a good idea to be honest.

@gtca
Copy link
Collaborator

gtca commented May 19, 2021

Thanks for the reference. I am mostly concerned at the moment about breaking some building tools that still might rely on it.
But that's a good point. Should we adopt something like anndata/setup.py?

@ilia-kats
Copy link
Collaborator

I think pretty much the only people who are still using setup.py are Linux distro packagers, who package Python packages for the distro repositories (this is what most of the discussion in the referenced issue revolves around). Since most of our dependencies are not in distro repos (e.g. Debian has anndata, but not scanpy, Arch has both only in the unofficial AUR) or severely oudated (Debian's umap-learn is still 0.4.5 vs. 0.5.1 upstream, Arch's unoffical scanpy is 1.6.0 vs. 1.7.2 upstream) and muon isn't really on any distribution's radar yet, I don't see it being packaged by distros any time soon. The setuptools people seem pretty adamant that setuptools is buggy, unmaintained, and should not be used, so by the time someone wants to package muon, I believe they will have already transitioned to a new workflow not involving setuptools.

If someone insists on using the setup.py way of installing stuff on their own machine, I think there are some tools that can automatically create a setup.py from the pyproject.toml (I guess we should also mention it in the docs). My feeling is, having a setup.py in the repo will encourage people to use something that will go away in the future instead of nudging them towards the future-proof way.

But I leave the final decision up to you.

@gtca
Copy link
Collaborator

gtca commented May 20, 2021

Thank you for the elaborate reasoning, @ilia-kats!

These seem like good points to me, we will then work on it in another PR that should tackle at least

  • proper pyproject.toml,
  • detailed installation instructions in the docs
  • with alternatives to setup.py install and setup.py develop outlined.

Apparently the latter might require a relatively recent pip.

@gtca gtca merged commit 3744448 into master May 20, 2021
@gtca gtca deleted the fix-setup branch May 26, 2021 09:22
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