Skip to content

gh-117404: Add structured version info for compression modules #117405

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

gh-117404: Add structured version info for compression modules #117405

wants to merge 2 commits into from

Conversation

serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented Mar 31, 2024 •
edited by github-actions bot
Loading

  • Add named tuples zlib_version and zlib_runtime_version in the zlib module.
  • Add string BZLIB_VERSION and named tuple bzlib_version in the bz2 module.
  • Add strings LZMA_VERSION and LZMA_RUNTIME_VERSION and named tuples lzma_version and lzma_runtime_version in the lzma module.

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

@serhiy-storchaka
pythongh-117404: Add structured version info for compression modules
9d8694c 
* Add named tuples zlib_version and zlib_runtime_version in the zlib
  module.
* Add string BZLIB_VERSION and named tuple bzlib_version in the bz2
  module.
* Add strings LZMA_VERSION and LZMA_RUNTIME_VERSION and named tuples
  lzma_version and lzma_runtime_version in the lzma module.
@bedevere-app bedevere-app bot mentioned this pull request Mar 31, 2024
Open
@gpshead
Copy link
Member

gpshead commented Mar 31, 2024

How about we shorten the names, there is no need to repeat the module name in the name of the constant.

@ned-deily
Copy link
Member

How about we shorten the names, there is no need to repeat the module name in the name of the constant.

Perhaps we should use the same prefixes we use for the corresponding configure options, i.e. ZLIB, BZIP2, and LIBLZMA.

  ZLIB_CFLAGS C compiler flags for ZLIB, overriding pkg-config
  ZLIB_LIBS   linker flags for ZLIB, overriding pkg-config
  BZIP2_CFLAGS
              C compiler flags for BZIP2, overriding pkg-config
  BZIP2_LIBS  linker flags for BZIP2, overriding pkg-config
  LIBLZMA_CFLAGS
              C compiler flags for LIBLZMA, overriding pkg-config
  LIBLZMA_LIBS
              linker flags for LIBLZMA, overriding pkg-config

@gpshead
Copy link
Member

gpshead commented Apr 1, 2024

Perhaps we should use the same prefixes we use for the corresponding configure options, i.e. ZLIB, BZIP2, and LIBLZMA.

That'd take us in the wrong direction. These are for use by people writing Python code, not for use by people who build CPython. lzma.version -> (2,4,6) is clear and concise. lzma.LZMA_VERSION is needlessly repetitive.

this is a bikeshed, we could also argue that constants must always be in CAPS and ask for lzma.VERSION or for the lower case one to be a callable lzma.version(). I don't think it matters that much. I lean towards make the most likely to be used and thing people want one be the easiest to type. But I'd accept any, these aren't worth arguing over. The few people who need them will use them under whatever names we provide them as.

@serhiy-storchaka
Copy link
Member Author

Let continue the main design discussion on the issue, and only use PR for discussing minor implementation details.

merwok
merwok reviewed Apr 9, 2025
View reviewed changes
Doc/library/bz2.rst

.. data:: BZLIB_VERSION

The version string of the bzlib library.
Copy link
Member

Choose a reason for hiding this comment

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

I haven’t seen bz2 called bzlib before. It’s always bzip2, bz2-lib or libbz2.

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

@merwok merwok merwok left review comments

@gpshead gpshead Awaiting requested review from gpshead

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@serhiy-storchaka @gpshead @ned-deily @merwok