search

fplll / fpylll

A Python interface for https://github.com/fplll/fplll
GNU General Public License v2.0
118 stars 60 forks source link

bkz_param.pyx test fails if fplll directory contains the string "fplll" #233

Closed collares closed 2 years ago

collares commented 2 years ago

In NixOS, the default strategy file is installed to /nix/store/4r5dx5vqbhsz3vgm1060npd0nbsm8bqq-fplll-5.4.2/fplll/strategies/default.json, which confuses this test's assignment to the SAGE_LOCAL environment variable.

joerowell commented 2 years ago

This might be a bug I introduced with the "SAGE_LOCAL" fix earlier on... I'll take a look later on today.

On Fri, 27 May 2022, 23:29 Mauricio Collares, @.***> wrote:

In NixOS, the default strategy file is installed to /nix/store/4r5dx5vqbhsz3vgm1060npd0nbsm8bqq-fplll-5.4.2/fplll/strategies/default.json, which confuses this test's assignment to the SAGE_LOCAL environment variable https://github.com/fplll/fpylll/blob/bb265a59d1a167b1a4873fe2d84d996896cb2ea6/src/fpylll/fplll/bkz_param.pyx#L266 .

— Reply to this email directly, view it on GitHub https://github.com/fplll/fpylll/issues/233, or unsubscribe https://github.com/notifications/unsubscribe-auth/AF7CASALQO7NPZDDOI4VSJ3VMFEDFANCNFSM5XFTSI7Q . You are receiving this because you are subscribed to this thread.Message ID: @.***>

joerowell commented 2 years ago

Thanks for reporting this! Could you possibly check the code in #234 on your system to check that it fixes the issue?

collares commented 2 years ago

Tests now pass, thanks! By the way, I verified that this change also makes the tests pass (but it has different behavior when BKZ.DEFAULT_STRATEGY does not contain the string fplll at all):

diff --git a/src/fpylll/fplll/bkz_param.pyx b/src/fpylll/fplll/bkz_param.pyx
index f81ad20..1881691 100644
--- a/src/fpylll/fplll/bkz_param.pyx
+++ b/src/fpylll/fplll/bkz_param.pyx
@@ -263,7 +263,7 @@ def load_strategies_json(filename):

         >>> from fpylll import load_strategies_json, BKZ
         >>> from os import environ
-        >>> environ['SAGE_LOCAL'] = BKZ.DEFAULT_STRATEGY.decode().partition('fplll')[0]
+        >>> environ['SAGE_LOCAL'] = BKZ.DEFAULT_STRATEGY.decode().rpartition('fplll')[0]
         >>> strategies = load_strategies_json('default.json')
         >>> strategies[80].preprocessing_block_sizes
         (56,)

I trust you when you say that the doctest is redundant, though. Thanks again for taking a look at this!