Skip to content

[joss review] python requirements ? #5

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

Closed
thelfer opened this issue Apr 18, 2024 · 5 comments
Closed

[joss review] python requirements ? #5

thelfer opened this issue Apr 18, 2024 · 5 comments
Assignees
Labels
enhancement New feature or request

Comments

@thelfer
Copy link

thelfer commented Apr 18, 2024

[This is issue is part of the review of the paper submited to JOSS.]

First, I would recommend an INSTALL file at the root of the sources. On the first contact with the library, I didn't see any installation instructions and had to figure out that I had to use pip from the "Project links" section of the README.
The installation instructions are somehow hidden in the tutorial, which is is not so obvious. At least the INSTALL file could link to the tutorial, IMHO. Also the root of the directory contains file related to conda, which is not mentioned in the installation section.

Regarding first contact with pypeec, there are a few scripts called run... at the root of the sources which seems tools for developers. The prefix run seems misleading and users may be tempted to have a look at them. That is only an totally discardable advice, but maybe such scripts would be better in a dedicated subdirectory (something like tools for instance).

I followed your installation guide and it failed as follows:

(pypeec-env) $ python3 -m pip install pypeec
Collecting pypeec
  Could not find a version that satisfies the requirement pypeec (from versions: )
No matching distribution found for pypeec
(pypeec-env)$ python3 
Python 3.7.3 (default, Oct 11 2023, 09:51:27) 
[GCC 8.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.

I guess that my version of python is too old. Would you tell me some minimal requirements for pypeec ?

@otvam
Copy link
Owner

otvam commented Apr 19, 2024

Thank you very much for accepting this review and for the comments!

Concerning, the installation instruction, I will add a "INSTALL.md" file at the root of the repository. You are totally right that the "run_*.sh" are maintenance scripts that are only useful for the maintainers, so I will move them to a "scripts" directory.

For the installation, the supported Python version are ["3.9", "3.10", "3.11", "3.12"]. I know that several Linux distributions are still shipping older versions but some libraries used by PyPEEC requires a more recent version.

Probably, conda is the easiest way to create a environment with a compatible Python interpreter:

conda create -n pypeec python=3.10         # create the conda env with Python
conda activate pypeec                      # activate the environment
conda install -c conda-forge libstdcxx-ng  # the env might be missing this system library for the plots
python -m pip install pypeec               # install pypeec and the dependencies
pypeec -v                                  # check the installation

@thelfer
Copy link
Author

thelfer commented Apr 19, 2024

No problem. Thanks for taking my remarks into account.

For the installation, the supported Python version are ["3.9", "3.10", "3.11", "3.12"]. I know that several Linux distributions are still shipping older versions but some libraries used by PyPEEC requires a more recent version.

This shall be cleary stated in the INSTALL.md file. I did not check yet what is you CI procedure if any, but tested configurations is a very important information for end users (i.e. I would have known that I needed a more recent python version).

Probably, conda is the easiest way to create a environment with a compatible Python interpreter:

Thanks. My point was not to use conda, but that conda's usage was not documented. And I naively though that a conda package was available, i.e. that pip was not the only way of installing PyPEEC.

@otvam otvam self-assigned this Apr 21, 2024
@otvam otvam added the enhancement New feature or request label Apr 21, 2024
@otvam
Copy link
Owner

otvam commented Apr 21, 2024

Yes, ["3.9", "3.10", "3.11", "3.12"] are the versions tested during the CI.

The version requirements are now clearly stated in the INSTALL.md and in the documentation.

The problem to create a conda package is that the VTK (C++ library for 3D object manipulation) conda package is currently broken for win64 (#6). I might have found a workaround and will try to create a Conda package.

@thelfer
Copy link
Author

thelfer commented Apr 22, 2024

@otvam Thanks for the precision.
Concerning conda, don't be mistaken. My remark originates from the fact that there was no INSTALL file, so I had to somehow guess how to install it. I saw files related to conda at the root directory and though "maybe there is a conda package ?".
For the review, a conda package is not a matter, having clear install instructions is.
So don't bother with a conda package for the review (but feel free to do it if you find it important for the project)

@otvam
Copy link
Owner

otvam commented May 25, 2024

The conda package is now available through conda-forge!
It was overdue because the HPC libraries used by PyPEEC are much easier to install with conda.

@otvam otvam closed this as completed May 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants