I could fix the linter in one commit if wanted. And I would propose to use Black to not have any discussions around styling in the future. Most big projects are now moving towards that. e.g. Scikit-learn is using it.

@ofek could you have a quick look at the Hatchling config? This project is using a src layout and for some reason I cannot manage to properly import the package (I am installing with pip install .). If I remove the src folder then all works just fine. I tried to setup a blank project with the layout configuration option set to True and it was working fine. So I am not sure what is wrong here. Can it be due to the casing? Something else I am missing?

And another question, does it make sense to test the wheel? Or you think there is no reason to do so compared to just testing after doing pip install .?

Oh I got it to work doing build.targets.wheel.packages = ["src/SALib"]. I did not get that from the doc.

I just tried building from before those 2 commits and seems fine

I did not try to build before that, I was just installing with pip install -e . and then trying to do import SALib and this was failing. It's only with the last commit that the import was working. And now we see the tests are passing.

Oh wow you discovered a bug: salib vs SALib

I'll fix tonight

Oh wow you discovered a bug: salib vs SALib

I'll fix tonight

Yay glad I can help 😃

@willu47 @ConnectedSystems this is ready to be reviewed. The commit 0347916 (#527) is running the commit hooks hence the large diff. I suggest not looking at this commit when reviewing (in case you don't know, on GitHub you can filter commit's to review. Under conversation you have a all commits drop down). If you prefer I can remove this commit for now.

Please remove the explicit build.targets.wheel.packages and try https://github.com/pypa/hatch/releases/tag/hatchling-v1.8.1

Thank you @ofek for the fast update of Hatchling! It works just fine now. I tested locally and in the CI here. Thank you for having a look at this configuration and helping out.

Happy to help!

Thank you for the review @willu47!

I did the updates. For the documentation, I did already update the install instructions and added a section about creating an env with SALib installed in dev mode.

For the release process (then this should close #355), I added the steps at the end of the contributing doc following the last comment of the issue and adapting it slightly for this PR.

In general, I do think that the general structure and text could be updated. Though, I would suggest to separate theses changes and have them over #523 where I started to update the theme (which is ready on my side and needs inputs).

Hi @tupui

Tried to do a local install and test run and found these two issues:

1. Instructions on how to run tests need to updated. On that, should there be a script that supports hatch run tests or is simply pytest okay?
2. When running tests, I get a circular import error. I haven't dug deeply on it at all but my first guess is that this is due to new behavior that relies on SciPy in modules that did not previously require it.

I'll carve out some time during the week to look into it if you don't get to it first.

An example of one of the test errors:

Traceback:
..\..\..\miniconda3\envs\salib\lib\importlib\__init__.py:126: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
tests\test_sample_seed.py:5: in <module>
    from SALib.sample.morris import sample as morris_sampler
src\SALib\sample\morris\__init__.py:1: in <module>
    from .local import LocalOptimisation  # noqa: F401
src\SALib\sample\morris\local.py:7: in <module>
    from .strategy import Strategy
src\SALib\sample\morris\strategy.py:16: in <module>
    from scipy.spatial.distance import cdist  # type: ignore

[... snip ...]

ImportError: cannot import name '_iterative' from partially initialized module 'scipy.sparse.linalg.isolve'
(most likely due to a circular import) 
(C:\[path to SALib environment]\lib\site-packages\scipy\sparse\linalg\isolve\__init__.py)

Interesting I don't recall having issues there, and the CI is passing just fine. I would suspect an issue with your env and maybe try from scratch following what is done in the CI. I will try to reproduce on Monday.

Since we are using conda, there is no reason to me to use hatch run. My understanding of the command is that it would do something similar, ensuring you are using an env which matches your config. So yes just running pytest while being in a conda env is enough.

Okay, this is a weird one.

I run into an ImportError with SciPy due to a circular dependency

In [1]: import scipy
In [2]: scipy.stats

---------------------------------------------------------------------------
ImportError                               Traceback (most recent call last)
Input In [6], in <cell line: 1>()
----> 1 scipy.stats

File C:\[salib environment location]\lib\site-packages\scipy\__init__.py:211, in __getattr__(name)
    209 def __getattr__(name):
    210     if name in submodules:
--> 211         return _importlib.import_module(f'scipy.{name}')
    212     else:
    213         try:

# ... snip ...

ImportError: cannot import name '_iterative' from partially initialized module 'scipy.sparse.linalg.isolve' (most likely due to a circular import) (C:\[salib environment location]\lib\site-packages\scipy\sparse\linalg\isolve\__init__.py)

This is a fresh environment but will retry with another one. And maybe again on Linux, given the tests pass.

Sounds like your env is corrupted. Although from the traceback I am not sure if C:\[salib environment location] matches C:\programs\miniconda3\envs\salib.

Your PYTHONPATH is empty right?

False alarm - all working now.

Had to recreate the environment again after manually deleting the environment folder but its working now.

Great, let me know if there is more to do here 😃

All looks good to me so am merging now.

Thanks to all who took part, and again to @ofek for dropping by and helping out!