ci: check public API compatibility in pull requests by shrishsk · Pull Request #1033 · ni/nidaqmx-python

shrishsk · 2026-10-17T12:30:33Z

This contribution adheres to CONTRIBUTING.md.
~~I've updated CHANGELOG.md if applicable.~~ No client-facing library behavior changes.
I've added tests applicable for this pull request.

What does this Pull Request accomplish?

Adds automated API compatibility checks using Griffe.

Provides a separate reusable workflow, called from CI for pull requests and pushes to master and release branches.
Installs tools through an optional Poetry compatibility dependency group.
Uses NI's shared default Python version.
Defaults to the newest Git tag, with an explicit base SHA for release-target PRs.
Supports manual comparisons against a supplied version tag, branch, or commit.
Derives additional API visibility from structured source documentation instead of maintaining a package list.
Loads each package snapshot once and compares the root package once.
Emits safely escaped GitHub warning annotations for compatibility findings.
Documents the local reproduction and maintainer approval process.

Fixes #914.

Why should this Pull Request be merged?

Detects potential API-breaking changes before merging, rather than relying solely on internal tests after merge.

The documentation-derived policy covers documented APIs in private implementation modules and public aliases. Each snapshot uses its own documentation, preventing removal of documentation from hiding a previously documented API break.

Source exports and generated API files are unchanged. Export cleanup is a separate public API decision requiring maintainer agreement and would not retroactively repair historical release snapshots.

The breaking-change label allows maintainer-approved compatibility findings without hiding annotations. Operational failures remain fatal.

What testing has been done?

All 35 focused automated tests passed, including:
- DeviceCollection.device_names removal.
- Removal of nidaqmx.Task and gRPC session-option aliases while implementation classes remain.
- Deleted documentation and documented modules.
- Explicit :members: selections and :undoc-members: behavior.
- Alias chains, cycles, canonical mismatches, and unresolved targets.
- Ordinary external imports versus documented or publicly exported external aliases.
- Approval-label behavior, fatal operational errors, and escaped workflow-command output.
Formatting, targeted lint, type checking, spelling, and whitespace checks passed locally.
Workflow YAML/Bash syntax and Zizmor security checks passed.
Comparisons against 1.6.0 and the current upstream base passed.
Comparison against 1.3.0 completed with five expected compatibility findings.
The latest origin/master was merged without conflicts; all 35 tests and lockfile validation passed afterward.
Confirmed that generated API files and handwritten public exports are unchanged.

Hardware-dependent regression tests were not run because this change affects CI tooling, not library runtime behavior.

Signed-off-by: Shrish Kulkarni <shrish.kulkarni@ni.com>

zhindes

Concern about the maintainability of the PR yml and how to break compat if we need to.

bkeryan · 2026-10-19T16:20:25Z

+# API Compatibility
+
+Pull requests run Griffe to check the generated public API against the pull request's base commit.
+The check covers the package and the modules in the API reference separately because package


Is this something we should fix instead of working around?

bkeryan · 2026-10-19T16:30:30Z

+            nidaqmx.system.storage.persisted_task nidaqmx.system.system nidaqmx.system.watchdog \
+            nidaqmx.task nidaqmx.task.channels nidaqmx.task.collections nidaqmx.task.triggering \
+            nidaqmx.types nidaqmx.utils; do
+            if ! griffe check --search generated --against "$BASE_SHA" --format github "$module"; then


Good, --format github creates GitHub annotations. Can you please link to a workflow log that shows what these look like?

Also please post a screenshot, since the workflow log will expire and no longer be accessible.

Signed-off-by: Shrish Kulkarni <shrish.kulkarni@ni.com>

…pi-compatibility-checks Signed-off-by: Shrish Kulkarni <shrish.kulkarni@ni.com>

shrishsk · 2026-10-19T18:57:50Z

Updated the PR with Poetry-managed tools, the shared Python default, and a reusable workflow called from CI. It defaults to the latest tag and supports explicit bases and manual previous-release comparisons.

The manual package list is replaced by documentation-derived visibility, with regression tests. Approved breaking changes are documented; operational errors remain fatal.

@bkeryan, regarding fixing the underlying exports: would you prefer that cleanup in this PR or a follow-up? Updating current exports would improve consistency, but historical release comparisons would still need additional visibility handling.

Also, could you please approve the workflow run? Once an approved run demonstrates compatibility findings, I’ll post the log link and annotation screenshot.

bkeryan · 2026-10-19T19:46:33Z

@bkeryan, regarding fixing the underlying exports: would you prefer that cleanup in this PR or a follow-up? Updating current exports would improve consistency, but historical release comparisons would still need additional visibility handling.

follow-up PR

shrishsk · 2026-10-20T03:01:07Z

@bkeryan, regarding fixing the underlying exports: would you prefer that cleanup in this PR or a follow-up? Updating current exports would improve consistency, but historical release comparisons would still need additional visibility handling.

follow-up PR

@bkeryan @zhindes I feel the rest of the changes are in. Once the workflow is approved, I’ll post the log link and annotation screenshot.

bkeryan · 2026-10-20T17:05:44Z

@bkeryan, regarding fixing the underlying exports: would you prefer that cleanup in this PR or a follow-up? Updating current exports would improve consistency, but historical release comparisons would still need additional visibility handling.

follow-up PR

@bkeryan @zhindes I feel the rest of the changes are in. Once the workflow is approved, I’ll post the log link and annotation screenshot.

You can test out workflow changes in your personal fork of this repo.

bkeryan · 2026-10-20T18:19:38Z

Why are you doing this? It seems like you ran into some sort of problem, but you haven't clearly communicated what that problem is. Instead, you went off and wrote an elaborate workaround.

You wrote this in the PR description:

The documentation-derived policy covers documented APIs in private implementation modules and public aliases. Each snapshot uses its own documentation, preventing removal of documentation from hiding a previously documented API break.

I have no idea what policy you are talking about. Is this something you made up? Is this something that griffe implements? Is this something that was output from an LLM?

griffe is a tool that I have used before. Please use it in a normal way. If there are problems with doing that, please communicate what those problems are so that we can discuss how to solve them.

The GitHub issue includes an example of how to use griffe. I tried it again, and it still works:

PS C:\dev\nidaqmx-python> griffe check --search generated nidaqmx -a 1.3.0 --verbose generated\nidaqmx\stream_readers\_analog_multi_channel_reader.py:140: AnalogMultiChannelReader.read_waveforms(timeout): Parameter default was changed: Old: 10 New: 10.0 generated\nidaqmx\stream_readers\_analog_single_channel_reader.py:112: AnalogSingleChannelReader.read_waveform(timeout): Parameter default was changed: Old: 10 New: 10.0 generated\nidaqmx\stream_readers\_digital_multi_channel_reader.py:489: DigitalMultiChannelReader.read_waveforms(timeout): Parameter default was changed: Old: 10 New: 10.0 generated\nidaqmx\system\system.py:80: System.DriverVersion: Attribute value was changed: Old: collections.namedtuple('DriverVersion', ['major_version', 'minor_version', 'update_version']) New: nidaqmx.types.DriverVersion generated\nidaqmx\task\_task.py:364: Task.close: Public object points to a different kind of object: Old: function New: attribute

These are all intentional changes to the API signature that are not expected to break compatibility for normal use cases.

You're right that I should have explained the problem before adding the custom logic. I reproduced one case using Griffe 2.3.0: removing the documented DeviceCollection.device_names property produced no findings with stock Griffe, while my custom pass detected the removal. Do you think this edge case is important enough to justify the extra logic? If not, I'm happy to remove the custom script and use Griffe normally.

bkeryan · 2026-10-20T18:28:33Z

+    permissions:
+      contents: read
+    with:
+      base: ${{ github.event_name == 'pull_request' && startsWith(github.base_ref, 'releases/') && github.event.pull_request.base.sha || '' }}


For CI runs triggered on push, I think the base branch is probably specified by github.ref_name. Please test this out in your personal fork of the repo and link to the resulting workflow log.

https://docs.github.com/en/actions/reference/workflows-and-actions/contexts#github-context

bkeryan · 2026-10-20T18:32:00Z

    branches:
      - master
      - 'releases/**'
+    types: [opened, synchronize, reopened, edited, labeled, unlabeled]


Why are you doing this?

I listed these so CI reruns when it matters: opened, synchronize, and reopened cover the usual PR lifecycle; edited catches a change to the target branch, which can change the comparison baseline (though title/body edits will rerun it too); and labeled/unlabeled recheck the result when the breaking-change label changes. If we decide the custom compatibility or label behavior isn’t needed, I’ll remove the extra triggers too.

ci: check public API compatibility in pull requests

d337682

Signed-off-by: Shrish Kulkarni <shrish.kulkarni@ni.com>

shrishsk requested review from bkeryan and zhindes as code owners October 17, 2026 12:30

shrishsk marked this pull request as draft October 17, 2026 12:32

ci: cover documented nested APIs in compatibility checks

3e0d95a

Signed-off-by: Shrish Kulkarni <shrish.kulkarni@ni.com>

shrishsk marked this pull request as ready for review October 17, 2026 14:10

zhindes requested changes Oct 19, 2026

View reviewed changes

Comment thread .github/workflows/PR.yml Outdated

Comment thread CONTRIBUTING.md Outdated

bkeryan requested changes Oct 19, 2026

View reviewed changes

Shrish Kulkarni added 2 commits October 20, 2026 00:10

ci: check documented API compatibility with reusable workflow

296cf88

Signed-off-by: Shrish Kulkarni <shrish.kulkarni@ni.com>

Merge remote-tracking branch 'origin/master' into users/kshrish/914-a…

161c874

…pi-compatibility-checks Signed-off-by: Shrish Kulkarni <shrish.kulkarni@ni.com>

bkeryan requested changes Oct 20, 2026

View reviewed changes

Shrish Kulkarni added 2 commits October 22, 2026 17:28

ci: refine API compatibility checks

6842629

ci: remove checker unittest suite

b948f67

Conversation

shrishsk commented Oct 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this Pull Request accomplish?

Why should this Pull Request be merged?

What testing has been done?

Uh oh!

zhindes left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bkeryan Oct 19, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bkeryan Oct 19, 2026

Choose a reason for hiding this comment

Uh oh!

bkeryan Oct 19, 2026

Choose a reason for hiding this comment

Uh oh!

shrishsk commented Oct 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bkeryan commented Oct 19, 2026

Uh oh!

shrishsk commented Oct 20, 2026

Uh oh!

bkeryan commented Oct 20, 2026

Uh oh!

bkeryan Oct 20, 2026 • 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!

shrishsk Oct 22, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

bkeryan Oct 20, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bkeryan Oct 20, 2026

Choose a reason for hiding this comment

Uh oh!

shrishsk Oct 22, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

shrishsk commented Oct 17, 2026 •

edited

Loading

shrishsk commented Oct 19, 2026 •

edited

Loading

bkeryan Oct 20, 2026 •

edited

Loading