[python-ldap] Add type hints by Alphix · Pull Request #522 · python-ldap/python-ldap · GitHub

With this patch applied, MYPYPATH=/Stubs mypy --strict Lib/ passes without any warnings. tox also passes for everything from py37 to py11 (and also including doc and pypy3).

I've tried my best to not make more modifications than strictly necessary. Most of the time, changes merely serve to make life easier for mypy (i.e. to enable type inference) or to fixup non-type aware code.

The patch might look dauntingly large, but once you cut away the trivial stuff (untyped function definitions which have been replaced with types ones, lots and lots of new import statements, etc) the diff is actually not that big.

With regard to the CI failures, I should add that py36 fails even without this patch applied. And pypy passes on my machine, the failure seems related to how the CI is configured...

With this patch applied:

~/checkouts/python-ldap$ tox
...
  py36: FAIL code -11 (0.50 seconds)
  py37: OK (14.72=setup[4.62]+cmd[10.10] seconds)
  py38: OK (13.79=setup[3.64]+cmd[10.14] seconds)
  py39: OK (14.70=setup[4.61]+cmd[10.09] seconds)
  py310: OK (14.46=setup[4.70]+cmd[9.76] seconds)
  py311: OK (12.52=setup[3.19]+cmd[9.33] seconds)
  c90-py36: FAIL code 1 (3.13 seconds)
  c90-py37: OK (15.26=setup[5.13]+cmd[10.12] seconds)
  py3-nosasltls: OK (10.38=setup[0.01]+cmd[0.11,1.48,0.29,8.50] seconds)
  doc: OK (9.03=setup[4.59]+cmd[0.13,0.04,3.01,1.26] seconds)
  py3-trace: OK (13.60=setup[4.33]+cmd[9.27] seconds)
  pypy3: OK (17.25=setup[6.45]+cmd[10.80] seconds)
  evaluation failed :( (139.37 seconds)

~/checkouts/python-ldap$ MYPYPATH=Stubs/ mypy --strict Lib
mypy.ini: No [mypy] section in config file
Success: no issues found in 40 source files

Hi, thanks for the PR!

Looks good, would you be able to split it into a few more commits each doing self-contained changes: any fixes, ldap.pkginfo changes, compat.py removal, etc. in any order that makes sense and then just adding the type hints only?

And just out of curiosity, is there a reason you chose to use <type> | None rather than Optional[<type>]?

Excellent work! A few comments and suggestions to push your PR over the finish line:

Fix Python 3.6 compatibility issues. We still support old Python versions for $REASONS..., sorry.
Please don't introduce yet another top-level module name. We like to move ldapurl and ldif into the ldap package eventually. I suggest ldap.types module.
Don't use from ldap_types import *. Star-imports are frowned upon and make code harder to read.
The PR is missing a PEP 561 type marker file. Add a Lib/ldap/py.typed file and add the file to MANIFEST.in
Move mypy.ini content to either pyproject.toml or setup.cfg
Extend tox.ini with a mypy check for every supported Python version
Add _ldap.pyi to MANIFEST.in
Would it make sense to move _ldap.pyi to Lib/_ldap.pyi and install it along the shared extension?

Python 3.6 fails because it does not support future annotations:

     File "/home/runner/work/python-ldap/python-ldap/.tox/c90-py36/lib/python3.6/site-packages/ldap/__init__.py", line 6
      from __future__ import annotations
                                       ^
  SyntaxError: future feature annotations is not defined

Hi! Why has this this work stalled? @Alphix are you still interested in continuing this?

Fix Python 3.6 compatibility issues. We still support old Python versions for $REASONS..., sorry.

What are those reasons? Many projects have already dropped 3.7.

Hi! Why has this this work stalled? @Alphix are you still interested in continuing this?

@intgr - it's the usual reason, I need to find the time to take the comments into account and update the patch. Maybe next week, we'll see...

Fix Python 3.6 compatibility issues. We still support old Python versions for $REASONS..., sorry.

What are those reasons? Many projects have already dropped 3.7.

My problem is that tox py36 fails even without the patch applied, haven't looked into that yet...

Fix Python 3.6 compatibility issues. We still support old Python versions for $REASONS..., sorry.

What are those reasons? Many projects have already dropped 3.7.

Because we (downstream vendor) still support Python 3.6.

Because we (downstream vendor) still support Python 3.6.

My question was: why can't python-ldap drop support for Python 3.6 (and maybe 3.7)?

Because we (downstream vendor) still support Python 3.6.

My question was: why can't python-ldap drop support for Python 3.6 (and maybe 3.7)?

My guess would be RHEL?

Anyway, does tox py36 work for anyone else? Without this patch applied? Because it doesn't for me....

Because we (downstream vendor) still support Python 3.6.

My question was: why can't python-ldap drop support for Python 3.6 (and maybe 3.7)?

My guess would be RHEL?

Anyway, does tox py36 work for anyone else? Without this patch applied? Because it doesn't for me....

tox -e py36 is passing on my Fedora machine. It's also working fine on our CI, which runs the tests both on Ubuntu and Fedora.

Looks good, would you be able to split it into a few more commits each doing self-contained changes: any fixes, ldap.pkginfo changes, compat.py removal, etc. in any order that makes sense and then just adding the type hints only?

Sorry, but I think that would just be way too much work at the moment. The changes to the code mostly go hand-in-hand with the typing annotations (and in most cases are just there to make the types more explicit to mypy).

And just out of curiosity, is there a reason you chose to use <type> | None rather than Optional[<type>]?

That was an oversight, didn't realise it was relevant for backwards compatibility, I've fixed that in the second version.

Excellent work! A few comments and suggestions to push your PR over the finish line:

Fix Python 3.6 compatibility issues. We still support old Python versions for $REASONS..., sorry.

Please don't introduce yet another top-level module name. We like to move ldapurl and ldif into the ldap package eventually. I suggest ldap.types module.

Don't use from ldap_types import *. Star-imports are frowned upon and make code harder to read.

The PR is missing a PEP 561 type marker file. Add a Lib/ldap/py.typed file and add the file to MANIFEST.in

Move mypy.ini content to either pyproject.toml or setup.cfg

Extend tox.ini with a mypy check for every supported Python version

Add _ldap.pyi to MANIFEST.in

Would it make sense to move _ldap.pyi to Lib/_ldap.pyi and install it along the shared extension?

@tiran : I think I've addressed all the points you raised (and the CI seems happy). The only thing I can think of is that mypy is not enabled for the py312 environment right now because it needs different compile flags. Not sure what the best way to handle that in tox.ini would be....but perhaps it could be addressed in a separate patch?

Also, I should add that this is just a starting point for type annotations. There are still a bunch of uses of Any and type: ignore in the code. I already have another branch which gradually fixes up those and makes the type checking stricter (but that's for later).

Looks good, would you be able to split it into a few more commits each doing self-contained changes: any fixes, ldap.pkginfo changes, compat.py removal, etc. in any order that makes sense and then just adding the type hints only?

Sorry, but I think that would just be way too much work at the moment. The changes to the code mostly go hand-in-hand with the typing annotations (and in most cases are just there to make the types more explicit to mypy).

Scratch that, I am working on splitting the PR into several preparatory patches, one big type hints patch, and one final patch integrating type checking (and learning a lot about git add -p in the process).

So you might want to hold off on the review for now...

Looks good, would you be able to split it into a few more commits each doing self-contained changes: any fixes, ldap.pkginfo changes, compat.py removal, etc. in any order that makes sense and then just adding the type hints only?

Ok, done. I've split the patch into a bunch of commits doing a lot of prep work, then one big patch adding the type hints only and finally one patch integrating mypy into the build machinery.

Awesome, thanks for your work. I tried it out in my project and have some feedback.

Ok, I've pushed a new version taking @intgr's comments into account. Most controversial change here is the move of _ldap to ldap._ldap (which is why that's a separate patch)

@tiran gentle ping?

Hey, @Alphix, can you update this to resolve conflicts, and then page @droideck, @mistotebe, and @tiran for review?

Thanks so much... this is a tremendous contribution that shouldn't be left to bit-rot.

Hey, @Alphix, can you update this to resolve conflicts, and then page @droideck, @mistotebe, and @tiran for review?

@macserv Eh, I'm a volunteer, using my free time for this. If you want to push this forward, I'd suggest you try to get in touch with the maintainers...once there's some sign of interest from their side, I'd be happy to rebase and resolve conflicts...

@Alphix Sorry for the late reply! Maintainer is here!
Thank you for your contribution!

If you are still around, could you please rebase it?

@Alphix Sorry for the late reply! Maintainer is here! Thank you for your contribution!

If you are still around, could you please rebase it?

@droideck: will do 🙋, I'm on vacation right now, but hopefully I'll have time next week for a rebase

Most controversial change here is the move of _ldap to ldap._ldap (which is why that's a separate patch)

Why is this necessary?
The commit message says:

This is necessary in order for type checking due to:
python/typing#1333

Given that _ldap is an internal module, this change is hopefully ok.

That upstream issue is still open.

Most controversial change here is the move of _ldap to ldap._ldap (which is why that's a separate patch)

Why is this necessary? The commit message says:

This is necessary in order for type checking due to:
python/typing#1333
Given that _ldap is an internal module, this change is hopefully ok.

That upstream issue is still open.

Not sure I understand the question? Is the commit message unclear?

Avoid redefining variables to keep type checkers happy.

First, the criticality in class SyncRequestControl can be passed as an int/bool/etc, but it still makes sense (and helps type checkers) to make sure that it is actually stored as a bool. Second, some minor code changes to exclude the possibility that some variables are not None and to help type checkers understand the type of some objects. Last, make sure that the syncrepl_get_cookie() function is type-conformant.

__eq__ methods need to be able to handle being passed any kind of object. The remaining changes mostly serve to make it clearer to type checkers if/when a variable can/cannot be None.

Mostly some explicit None checks, avoiding variable redefinition and the removal of a circuitous import.

Mostly some code refactoring to avoid variable redefinition and to add some additional None checks. Make sure that _unparseChangeRecord can handle "bytes | List[bytes]" (since that will be part of the type definition of 3-tuple modifications). Also, change "valid_changetype_dict" to "valid_changetype_set" (since the variable is anyway used as a set, not as a dict).

Mostly some small changes to avoid variable redefinition and changing some functions to remove superfluous return values.

Instead of the magic loop in subentry.py which looks for appropriate classes from Lib/ldap/schema/models.py, let the classes register themselves explicitly in the SCHEMA_CLASS_MAPPING/SCHEMA_ATTR_MAPPING in order to not confuse type checkers (this should also be clearer to humans reading the code). In addition, add some more comments to subentry.py and do the usual type safety fixups (explicit type checks, asserts, avoiding variable redefinitions, etc).

First, rewrite extract_tokens() in Lib/ldap/schema/tokenizer.py as parse_tokens() and document the function. Then, use the new function in Lib/ldap/schema/models.py, and remove the token_defaults class attributes (which confuse type checkers as they can't decide which attributes a given class does/doesn't have) and instead set the attribute defaults explicitly. This might look like a big change, but most of it is repetitive changes throughout the classes in Lib/ldap/schema/models.py.

Small fix to keep type checkers happy.

Essentially, make sure that the cookie is always stored as binary

self.responseValue is bytes, not a str

…ntrols/* Several of the request controls are registered in KNOWN_RESPONSE_CONTROLS, which appears to be due to a bit too much copy-pasting.

Phew, after all the prep work, the actual type annotations can finally be added.

@Alphix I've updated your PR against master, can you double check you're happy with what I've done? If you can look over @tiran's comments and make sure you addressed all of them (it looks to me like you have) let me know and I am happy to merge afterwards.

I am a little bit unhappy about moving _ldap to ldap._ldap and that is also kind of an API change - but yes, it's a private module.
For some things I import _ldap.

Is this really necessary? for what? a mypy workaround?
Nowadays there is ty, which is also a very good type checker. Is that affected as well?

This is necessary in order for type checking due to: python/typing#1333 Given that _ldap is an internal module, this change is hopefully ok.

This helps in ensuring that the stub and the C module don't drift apart.

- Rebase type-annotations work onto current main (resolving conflicts) - Drop Python 3.6-3.8 support: require Python >= 3.9 in pyproject.toml, update typing imports accordingly (remove typing_extensions shims) - Fix mypy --strict failures introduced by updated pyasn1 stubs - Fix AnyStr usage in syncrepl.py (replace with Union[str, bytes]) - Fix Dict key type in OpenLDAPSyncreplCookie._csnset (was Dict[int,str]) - Add class-level `responseName: Optional[str]` to ExtendedResponse so the attribute is accessible on the class object in ldapobject.py - Add class-level `SCHEMADIR: Optional[str]` annotation in _slapdtest.py - Make uri parameter Optional[str] in SimpleLDAPObject and ReconnectLDAPObject to match main's default-None usage - Add types-pyasn1 to tox mypy environment deps Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

threading has been a mandatory module since Python 3.7 (PEP 528).

Python 3.10 introduced the X | Y union syntax (PEP 604) and Optional[X] is now considered legacy style. Since the project requires Python >= 3.9, add 'from __future__ import annotations' (PEP 563) to every affected module so the new syntax is valid at runtime on 3.9 as well.

constants.py is already the canonical source for the constants exposed by the C extension (it drives constants_generated.h). Add a generate_pyi() function alongside the existing print_header(), so that the constants and error-class sections of _ldap.pyi can be regenerated whenever the native interface changes: python Lib/ldap/constants.py --pyi

Older versions of types-pyasn1 define SequenceOfAndSetOfBase.componentType as Optional[NamedTypes], causing mypy to reject assignments of concrete ASN.1 types in SequenceOf/SetOf subclasses. Newer mypy/types-pyasn1 (e.g. mypy 1.15) fix the stubs and consider such type:ignore comments unused. Add # type: ignore[assignment] to the five affected assignments, and set warn_unused_ignores = False per-module in setup.cfg so newer mypy does not reject the comments that older CI mypy still needs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Is this really necessary? for what? a mypy workaround?

I must admit that my memory is a bit (very) foggy given the age of the PR, but I think you already provided the same question (#522 (comment)) and I replied to it (#522 (comment)).

Nowadays there is ty, which is also a very good type checker. Is that affected as well?

No clue, feel free to test it yourself and share your findings?

@Alphix I've updated your PR against master, can you double check you're happy with what I've done?

@mistotebe I've had a quick look, and it seems ok to me.

If you can look over @tiran's comments and make sure you addressed all of them (it looks to me like you have) let me know and I am happy to merge afterwards.

Yeah, as far as I know I addressed them back in 2024, and I don't think @tiran has said anything to indicate that there's lingering issues...

I am a little bit unhappy about moving _ldap to ldap._ldap and that is also kind of an API change - but yes, it's a private module. For some things I import _ldap.

Hmm, can you share what's in _ldap that you prefer over the ldap interfaces? Are you accessing _ldap.LDAP's methods directly?

Overall we'd like to move most modules (ldif, ldapurl etc.) under ldap, because the pypi namespace is polluted with unrelated modules and this is causing real (discoverability) issues: e.g. pypi install ldif might not give you what you wanted.

We maintain a linux distribution based on Debian 12 and Debian 13 (and earlier releases).
We are heavy users of python-ldap and rely on it's functionality even during package upgrades or Debian release upgrades.
There is a time-frame in which the Debian packages are half-unpacked and a import of Python-ldap not available - e.g. in asynchronous components or scripts running in Debian pre-installation maintainer scripts.
When moving library imports, we observed race-conditions where the import gets non-functional for a short amount of time.
We didn't have this kind of problems with python-ldap yet, but with other not so critical libraries.

Also we are very happy at the moment, that we could drop all self-maintained patches for python-ldap and use the upstream packages from Debian again.
Especially because of securty monitoring - each forked package adds a burden.
This will change with #618 again :-(

And we really need the #466 for performance, security and correctness reasons.
As we are going to fork python-ldap again, we will include this now manually again.

Semantic versioning wants for breaking changes a major version update, for features a minor version increase and for fixes a patchlevel increase.

So, I would consider the VLV thing a bugfix with a patchlevel increase.
The DN string is a feature, which would be a minor update.
And the moving of these libraries is a breaking change - major update.

I am not sure, if Debian would just integrate the breaking change - if it looks like a non-major-change version number - into a point release. That would be much effort for us.

So, I would be happy, if such library moves would still provide an backwards compatible alias:
_ldap.py → from ldap._ldap import *
ldif.py → from ldap.ldif import *

python-ldap doesn't seem to follow semantic versioning.
(So I also don't see why the DN feature - which is a completely backwards compatible feature/not changing any semantics - cannot be taken in one of the next releases.)

Alphix mentioned this pull request May 16, 2023

Add PEP 484 -- Type Hints #368

Open

tiran requested changes Jul 21, 2023

View reviewed changes

Alphix force-pushed the type-hints-hack branch 2 times, most recently from 41ffdf3 to 8b4736f Compare January 24, 2024 15:02

Alphix force-pushed the type-hints-hack branch from 8b4736f to e203e04 Compare January 27, 2024 17:04

intgr reviewed Jan 29, 2024

View reviewed changes

Comment thread Lib/ldap/controls/libldap.py Outdated

Comment thread Lib/ldap/types.py Outdated

Comment thread Lib/_ldap.pyi Outdated

Comment thread Lib/_ldap.pyi Outdated

Comment thread setup.py Outdated

Alphix requested a review from tiran January 29, 2024 19:40

Alphix force-pushed the type-hints-hack branch from e203e04 to ba337e8 Compare January 30, 2024 20:46

spaceone reviewed Aug 7, 2025

View reviewed changes

Comment thread Lib/ldap/controls/libldap.py Outdated

Alphix force-pushed the type-hints-hack branch from ba337e8 to 2726abc Compare August 14, 2025 22:14

Alphix added 14 commits June 25, 2026 14:47

[python-ldap] Type fixes for Lib/ldap/sasl.py

a07777b

Avoid redefining variables to keep type checkers happy.

[python-ldap] Type fixes for Lib/ldapurl.py

7dbcee0

__eq__ methods need to be able to handle being passed any kind of object. The remaining changes mostly serve to make it clearer to type checkers if/when a variable can/cannot be None.

[python-ldap] Type fixes for Lib/slaptest/*

4f21edb

Mostly some explicit None checks, avoiding variable redefinition and the removal of a circuitous import.

[python-ldap] Type fixes for Lib/ldap/ldapobject.py

e8f728e

Mostly some small changes to avoid variable redefinition and changing some functions to remove superfluous return values.

[python-ldap] Type fixes for Lib/ldap/controls/openldap.py

25f6463

Small fix to keep type checkers happy.

[python-ldap] Type fixes for Lib/ldap/controls/pagedresults.py

d4776e4

Essentially, make sure that the cookie is always stored as binary

[python-ldap] Type fixes for Lib/ldap/extop/__init__.py

6bd518a

self.responseValue is bytes, not a str

[python-ldap] Improve documentation in Lib/ldap/dn.py

1595544

[python-ldap] Fix KNOWN_RESPONSE_CONTROLS registration in Lib/ldap/co…

b74009b

…ntrols/* Several of the request controls are registered in KNOWN_RESPONSE_CONTROLS, which appears to be due to a bit too much copy-pasting.

[python-ldap] Add type annotations

3ffecbb

Phew, after all the prep work, the actual type annotations can finally be added.

mistotebe force-pushed the type-hints-hack branch from ebf5da0 to 258ec46 Compare July 9, 2026 12:34

Alphix and others added 8 commits July 9, 2026 14:10

[python-ldap] Integrate typing into the build/CI system

fbb9d96

[python-ldap] Move _ldap to ldap._ldap

96615b7

This is necessary in order for type checking due to: python/typing#1333 Given that _ldap is an internal module, this change is hopefully ok.

[python-ldap] Add stubtest to typing CI

b4f61cf

This helps in ensuring that the stub and the C module don't drift apart.

[python-ldap] Remove threading fallback

1cd9d43

threading has been a mandatory module since Python 3.7 (PEP 528).

mistotebe force-pushed the type-hints-hack branch from 258ec46 to 5cba2fb Compare July 9, 2026 14:01

[python-ldap] Add type hints by Alphix · Pull Request #522 · python-ldap/python-ldap · GitHub

Uh oh!

[python-ldap] Add type hints - #522

[python-ldap] Add type hints#522Alphix wants to merge 35 commits intopython-ldap:mainpython-ldap/python-ldap:mainfrom Alphix:type-hints-hackAlphix/python-ldap:type-hints-hackCopy head branch name to clipboard

Conversation

Alphix commented May 15, 2023

Uh oh!

Alphix commented May 16, 2023

Uh oh!

mistotebe commented Jul 21, 2023

Uh oh!

tiran left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

intgr commented Oct 1, 2023

Uh oh!

Alphix commented Oct 1, 2023

Uh oh!

tiran commented Oct 2, 2023

Uh oh!

intgr commented Oct 2, 2023

Uh oh!

Alphix commented Oct 5, 2023

Uh oh!

tiran commented Oct 6, 2023

Uh oh!

Alphix commented Jan 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Alphix commented Jan 24, 2024

Uh oh!

Alphix commented Jan 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Alphix commented Jan 27, 2024

Uh oh!

intgr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Alphix commented Jan 30, 2024

Uh oh!

Alphix commented Apr 3, 2024

Uh oh!

macserv commented Nov 27, 2024

Uh oh!

Alphix commented Nov 28, 2024

Uh oh!

droideck commented Aug 6, 2025

Uh oh!

Alphix commented Aug 6, 2025

Uh oh!

spaceone commented Aug 7, 2025

Uh oh!

Uh oh!

Alphix commented Aug 14, 2025

Uh oh!

mistotebe commented Jul 9, 2026

Uh oh!

spaceone commented Jul 9, 2026

Uh oh!

Alphix commented Jul 9, 2026

Uh oh!

Alphix commented Jul 9, 2026

Uh oh!

mistotebe commented Jul 10, 2026

Uh oh!

spaceone commented Jul 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

[python-ldap] Add type hints#522
Alphix wants to merge 35 commits into
python-ldap:mainfrom
Alphix:type-hints-hack

tiran left a comment •
edited
Loading

Alphix commented Jan 24, 2024 •
edited
Loading

Alphix commented Jan 26, 2024 •
edited
Loading