gh-132983: Split _zstd_set_c_parameters

[AA-Turner]

The current _zstd_set_c_parameters shares little between the two possible (int or dict) paths. By splitting the function, we can now use the Py_ssize_t AC converter.

One slight change worth noting is that the constructor won't raise an error when level is $-2^{63}$ but will just silently use the default. cc @erlend-aasland if there's a better method/sentinel we can use in the clinic to detect if the value has been set or not.

>>> _zstd.ZstdCompressor(level=-(1<<63)+1)
Traceback (most recent call last):
  File "<python-input-17>", line 1, in <module>
    _zstd.ZstdCompressor(level=-(1<<63)+1)
    ~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^
ValueError: compression level -9223372036854775807 not in valid range -131072 <= level <= 22.
>>> _zstd.ZstdCompressor(level=-(1<<63))
<compression.zstd.ZstdCompressor object at 0x7f61875b2210>

I also think it might be worth considering removing the level XOR options check, as we could adopt the rule that a compression level set in an options dict overrides the level parameter. I can see reasonable arguments to keep the status quo, however, so I've not done this yet.

A

cc @Rogdham

• Issue: Implement PEP 784 - Adding Zstandard to the Python standard library #132983

[serhiy-storchaka]

if there's a better method/sentinel we can use in the clinic to detect if the value has been set or not

No, there is not. You cannot distinguish the passed value from the default value. It is better to not using Argument Clinic for conversion of this parameter.

BTW, Py_ssize_t is the same as int on 32-bit platforms, so there is no good reason to use it here.

[bedevere-app]

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

[emmatyping]

A few suggestions for wording of error messages, but I think otherwise this looks great. Thank you for refactoring this!

[emmatyping]

Thanks, this is a great refactor!

resolved

[serhiy-storchaka]

I am not sure about term "illegal", this is a question to native speakers, but the rest LGTM. Thank you.

[emmatyping]

I am not sure about term "illegal"

It's not wrong, but perhaps invalid is better?

[miss-islington-app]

Thanks @AA-Turner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

[bedevere-app]

GH-134838 is a backport of this pull request to the 3.14 branch.