search

AFM-SPM / TopoStats

An AFM image analysis program to batch process data and obtain statistics from images
https://afm-spm.github.io/TopoStats/
GNU Lesser General Public License v3.0
60 stars 11 forks source link

package: Unpin topoly version #995

Closed ns-rse closed 3 days ago

ns-rse commented 1 week ago

Wheels are now available for all OS's and architectures (thanks @prubach :+1: )

Closes #994

ns-rse commented 1 week ago

Rebase this branch after #988 has been merged to resolve the issues with pytest-regtest under OSX.

ns-rse commented 1 week ago

I've manually updated the necessary files so that the change in nomenclature used by topoly to describe the connection types (? I don't even know what they are describing!) has changed from 4^2_1 > L4a1 and so forth.

Still feel we should be using pytest-regtest to handle these regression tests as its another thing to have to explain/teach how to do this manually for people who come in the future to work on the code base.

MaxGamill-Sheffield commented 1 week ago

Do we also need to pin numpy < 2 from Pawel's comment: "There seems to be some incompatibility with numpy 2.x. We built the 1.0.4 version before numpy 2.x was out and there are no issues with older numpy versions"

ns-rse commented 1 week ago

Do we also need to pin numpy < 2 from Pawel's comment: "There seems to be some incompatibility with numpy 2.x. We built the 1.0.4 version before numpy 2.x was out and there are no issues with older numpy versions"

Full comment for context (bold emphasis is mine)...

BTW while I was investigating that matter I found another issue when using certain functions in Topoly (mostly GLN). There seems to be some incompatibility with numpy 2.x. We built the 1.0.4 version before numpy 2.x was out and there are no issues with older numpy versions but since I didn't limit the version, when you now create a new virtual environment numpy 2.x is by default installed on newer versions of python. I have no clue about the cause as it results in some segfaults but only with certain input data, even though we don't use numpy data types etc. in the compiled code. I'm considering restricting the numpy versions for now but need to investigate it further.

Until we get bitten by whatever the problem is (and we don't have enough details to know what that is) I'd say no, not at the moment[^1].

Numpy 2.0.0 was a significant release, hence the major semantic version change with breakages to be expected and was made 5 months ago. There have been minor revisions since (current stable version is 2.1.3 released 2024-11-02). Long term, keeping up-to-date with revisions is important otherwise you end up in the situation when I first came to work on TopoStats with Docker containers running unsupported software.

For that reason I'm not a fan of pinning version dependencies for long periods, it kicks the can of fixing whatever is broken further down the line.

Further, and perhaps more fundamentally, if topoly do decide to pin the Numpy version then I would expect pip to resolve that for us and install the correct version of Numpy and other packages so that things "Just Work(TM)".

[^1]: Perhaps there is a hint as maybe explicit data types are required in some instances.