k-a-mendoza / Pyrrhenius

A python package for modeling mineral electric conductivity
MIT License
1 stars 0 forks source link

Suggest make pyfluids not optional in documentation as listed as a required dependency in setup.py #4

Open sgjholt opened 2 months ago

sgjholt commented 2 months ago

pyfluids in documentation

Screenshot 2024-09-01 at 2 58 48 PM

pyfluids in setup.py

Screenshot 2024-09-01 at 2 58 59 PM

Alt. Make pyfluids an optional dependency

I just modified the example in the setuptools doc page:

https://setuptools.pypa.io/en/latest/userguide/dependency_management.html#optional-dependencies.

I also strongly recommend you try to pin down the python, pandas and numpy versions, (even if only loosely). The latter two each have big (and code breaking) changes between v1 and v2 of their python apis. Not all users will understand how to debug this, so better just force.

setup(
    name="pyrrhenius",
    install_requires={
        numpy==*.*.*, 
        scipy==*.*.*,
        pandas==*.*.*,    
    },
    extras_require={
        "pyfluids": ["pyfluids"],
    },
)

Then the pip command would be

pip install pyrrhenius[pyfluids]

If you do make it optional, don't forget to add a try except on import and raise an error letting the user know that the required optional package isn't installed. I would try to separate everything that depends on that package into its own module, so that it isn't too strongly coupled with the code that relies on only the required modules.

sgjholt commented 2 months ago

NOTE: I also just noticed the pip install via git references a master branch, but you have only the main branch as default. I think you should change the end of the url to @main.

pip install git+git://github.com/k-a-mendoza/pyrrhenius@main