BoPeng / simuPOP

A general-purpose forward-time population genetics simulation environment.
http://bopeng.github.io/simuPOP/
GNU General Public License v2.0
30 stars 12 forks source link

Py3.9 deprecates the second argument to random.shuffle #117

Closed SimonGreenhill closed 7 months ago

SimonGreenhill commented 7 months ago

The method random.shuffle has a parameter random to allow users to specify a float in range [0.0, 1.0). However, this has been deprecated removed in 3.9 and removed in 3.11.

This means that running sampling.py:random_shuffle will fail with:

Traceback (most recent call last):
...
TypeError: Random.shuffle() takes 2 positional arguments but 3 were given

https://github.com/BoPeng/simuPOP/blob/b1ec05c7c683ca1b5c313babf82e5eff54683fe7/src/sampling.py#L94-L96

This is easily fixed by removing the second argument:

def random_shuffle(x):
    random.shuffle(x)

I can create a pull request to do this ... but I'm not sure whether removing this random argument makes a difference to the resulting shuffle (I think it just means you can't seed the random shuffle here).

See here for more details on the function, and more discussion here, and the code changes to python

BoPeng commented 7 months ago

Yes, by passing our own random number generator to random.shuffle we allow complete reproduction of simulations using our set seed function. Without this users will have to keep seeds for both python and simuPOP's RNGs.

I will be happy to merge a PR and make a minor release.

BoPeng commented 7 months ago

Yes, the test suite also fails for python 3.11

======================================================================
ERROR: testRandomSample (test_16_sampling.TestSampling.testRandomSample)
Testing random sampling (incomplete)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/bpeng/BoPeng/simuPOP/test/test_16_sampling.py", line 90, in testRandomSample
    s = drawRandomSample(self.pop, 10)
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/bpeng/anaconda3/envs/bioworkflows/lib/python3.11/site-packages/simuPOP-1.1.13-py3.11-macosx-10.9-x86_64.egg/simuPOP/sampling.py", line 227, in drawRandomSample
    return RandomSampler(sizes=sizes, subPops=subPops).drawSample(pop)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/bpeng/anaconda3/envs/bioworkflows/lib/python3.11/site-packages/simuPOP-1.1.13-py3.11-macosx-10.9-x86_64.egg/simuPOP/sampling.py", line 203, in drawSample
    random_shuffle(values)
  File "/Users/bpeng/anaconda3/envs/bioworkflows/lib/python3.11/site-packages/simuPOP-1.1.13-py3.11-macosx-10.9-x86_64.egg/simuPOP/sampling.py", line 96, in random_shuffle
    random.shuffle(x, getRNG().randUniform)
TypeError: Random.shuffle() takes 2 positional arguments but 3 were given

@SimonGreenhill Are you familiar with Github Actions? I would appreciate it very much if you could add CI to this repository so that we can easily detect such issues. The workflow should basically involve a Python environment with python setup.py install and python run_tests.py.

SimonGreenhill commented 7 months ago

yes -- here's the github actions: https://github.com/BoPeng/simuPOP/pull/118

BoPeng commented 7 months ago

Thanks. I merged the PR and will see if it works. I also created a PR #119 for the random.shuffle change since it is easy enough to do.

I will make a minor release after tests pass.

BoPeng commented 7 months ago

Test fails due to incompatibility with the latest version of boost.

2024-03-29T15:40:52.5449574Z ImportError: /opt/hostedtoolcache/Python/3.12.2/x64/lib/python3.12/site-packages/simuPOP-1.1.13-py3.12-linux-x86_64.egg/simuPOP/_simuPOP_ba.cpython-312-x86_64-linux-gnu.so: undefined symbol: _ZTIN5boost7archive6detail14basic_iarchiveE

Appears to be the same problem as https://github.com/espressomd/espresso/issues/3055

BoPeng commented 7 months ago

Thank @SimonGreenhill for reporting this issue and creating the github action file. I have merged the changes and released simuPOP version 1.1.14.