search

openforcefield / openff-toolkit

The Open Forcefield Toolkit provides implementations of the SMIRNOFF format, parameterization engine, and other tools. Documentation available at http://open-forcefield-toolkit.readthedocs.io
http://openforcefield.org
MIT License
301 stars 88 forks source link

Use new-ish NumPy random #1836

Closed mattwthompson closed 2 months ago

mattwthompson commented 3 months ago

Context https://docs.astral.sh/ruff/rules/numpy-legacy-random/#numpy-legacy-random-npy002

codecov[bot] commented 3 months ago

Codecov Report

Merging #1836 (4b19f96) into main (13bebaf) will not change coverage. The diff coverage is n/a.

Additional details and impacted files
j-wags commented 2 months ago

After looking at it a bit, I think the point of this (admittedly strange) test IS to run with different random inputs each time. So I don't think this change is needed, the (weird) test is (marginally) better without it, and the code is more readable in the old form.

But I am still bothered by how weird this test is. So an alternative to closing this PR would be making the test not-weird by hardcoding a random-ish conformer.

mattwthompson commented 2 months ago

Yeah it's a bit confusing, just trusting the "rules" here. It may never get removed but following their (admittedly weak) suggestion prevents this from being a confusing deprecation warning in the future