Skip to content

Conversation

@picnixz
Copy link
Member

@picnixz picnixz commented Nov 7, 2025

This one is much more tricky because Py_HASH_EXTERNAL was never documented and not really supported in the configure script. It would require user-defined defines and other hacks.

What I can do is remove the EXTERNAL docs and make a separate bugfix.


📚 Documentation preview 📚: https://cpython-previews--141205.org.readthedocs.build/


.. c:macro:: Py_HASH_CUTOFF
Buffers of length in range ``[1, Py_HASH_CUTOFF)`` are hashed using DJBX33A
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll need to check if Py_HASH_CUTOFF == 1 is actually effective (at first glance, no as the interval is empty). I just used the comment in the code.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, so with 1 it's actualoly useless. Considering it's really for advanced usage, I will not change the docs here and people will see that the interval in [1, 1) is empty. But the if branch will exist and will be compiled.

Copy link
Member

@ZeroIntensity ZeroIntensity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@picnixz picnixz requested a review from ZeroIntensity November 7, 2025 18:10
@picnixz
Copy link
Member Author

picnixz commented Nov 7, 2025

I decided to add the two blank lines around constants. Ideally I wanted sections, but I think two blank lines are better for reading the document. With so many versionadded, it's easy to get lost.

@ZeroIntensity
Copy link
Member

I already approved this, no need to request. I'm not too familiar with this area, so I'm basically just trusting that you know what you're doing in terms of content. The format and whatnot look fine.

@picnixz picnixz merged commit 3989e12 into python:main Nov 7, 2025
28 checks passed
@picnixz picnixz deleted the doc/capi/hash-141004 branch November 7, 2025 18:25
.. versionadded:: 3.4
Add :c:macro:`!Py_HASH_FNV` and :c:macro:`!Py_HASH_SIPHASH24`.

.. versionadded:: 3.13
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I messed up here... it's not 3.13 but it's 3.11...

The problem is that it's named SIPHASH13!!!!

@picnixz
Copy link
Member Author

picnixz commented Nov 7, 2025

Well.... I included a typo :D (but that's because we have confusing names!!) I will fix it tomorrow but it will be fast (so I won't backport it yet). I knew I shouldn't have left my dev session...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants