lattice-estimator icon indicating copy to clipboard operation
lattice-estimator copied to clipboard

Pin python requirements to specific versions

Open j2kun opened this issue 1 year ago • 5 comments

Continuing discussion from https://github.com/malb/lattice-estimator/pull/44

My expectation is that this does not cause any additional work, but that it will maintain consistent behavior of the project.

I suppose your concern is that if an update is pushed to one of sage's dependencies that has a semantic difference to the cryptography, then you might miss that update and this project would be out of date or become insecure, right?

At first glance, this seems to be an independent concern from this PR, because the dependencies currently listed requirements.txt appear to be mainly for style, the test harness, and documentation generation. The sage version does not appear to be specified anywhere (see, e.g., the test action). While I would personally recommend pinning sage's version, I can see the argument against it.

So at most this PR would help prevent changes in those tools from breaking the project's build and test systems.

I don't mind if you'd prefer to close this PR, though. It's your project :) It's just my habits that led me to do it in the original PR.

j2kun avatar Sep 22 '22 20:09 j2kun

Ah, actually I do see sagemath-standard==9.5 in the frozen requirements. Probably because I did sage -pip freeze, sorry, new to this sage stuff.

Perhaps it would be agreeable to pin as sagemath-standard>=9.5, which would give some of the same benefits?

j2kun avatar Sep 22 '22 20:09 j2kun

My nr 1 priority is that new contributors feel welcome and not dismissed out of hand :) My nr 2 priority is not create more work by aiming for production ready code in a research project. Let's go with "sagemath-standard>=9.5" and I'll then see what the fallout is

malb avatar Sep 23 '22 16:09 malb

Actually, doesn't sagemath-standard>=9.5 contradict fixing those other version numbers? This would render the requirements list contradictory if someone has sage 9.6 installed which will have its own requirements.

malb avatar Sep 23 '22 16:09 malb

Yeah, pip install will fail if sage releases 9.6 and it comes with additional dependencies that contradict the other dependencies in the list. However, sagemath-standard has a very small number of dependencies

sagemath-standard==9.5
  - cysignals [required: >=1.10.2]
    - Cython [required: >=0.28]

The reason those two aren't in the list of requirements in this PR is that sage's pip comes with them pre-installed as wheels. It appears that all the rest of the packages are dependencies of nbmake, sphinx, and pytest, which should not be impacted by sage updates.

j2kun avatar Sep 23 '22 16:09 j2kun

How about we do sagemath-standard>=9.5 and freeze the manual dependencies but let dependencies be dependencies? This seems to communicate intent better here?

malb avatar Sep 23 '22 20:09 malb