Repository navigation
Type the receivers of method calls from the packages' own annotations - #100
Open
MohammadHijjawi97 wants to merge 3 commits into
Open
MohammadHijjawi97 wants to merge 3 commits into
MohammadHijjawi97 wants to merge 3 commits into
Conversation
A changed method was matched only when the file names its class: on a class it imports, or on a value the file itself shows is an instance of one. Most calls on values a package returns went unflagged. With pandas 3.0.6, `DataFrame.applymap`, `NDFrame.swapaxes` and `DataFrame.groupby (axis=)` were in the diff while `df = pd.read_csv(...)` and `df.applymap(str)` were not flagged; with polars 2.0.0, `with_row_count`, `LazyFrame.melt` and `GroupBy.count` on frames from `pl.read_csv` and `pl.scan_csv`; with langchain-core 1.6.9, `BaseChatModel.predict` on `llm = ChatOpenAI(...)`, whose class lives in langchain-openai and inherits across packages, in a file that never imports langchain-core. The receivers are now typed from the pinned releases' own annotations, read statically with griffe (apidiff.receiver_map) and cached per release: a name assigned from a call gets the class the return annotation names, a chain types each step, annotated parameters and the file's own functions' returns count, and a class is read with its bases across the project's pinned packages, under every public path, up to the nearest class the diff says changed the member. scan_file records the evidence (FileUse.bound, accesses, assigned, annotated, chain_keywords); project.typed applies it; selection.type_project builds the maps for the trees the scan has and gives Engine.scan a typed copy of the project. A file whose values derive from a package it does not import uses that package's API too (FileUse.reaches, Project.code_use). Where the receiver cannot be typed, a member whose name is exactly one changed API's across the imported packages, and not a common Python name, counts by its name alone: the reports tag the use "[name match]", and its note goes into the block only with --include-name-matches. A class that became a function, or the reverse (pandas 3's option_context), is called the same way: a call of it is "uses this API" unless it passes a keyword the new signature does not take.
…of CI The review of the typed receivers found, on real pandas 3.0.6 and on synthetic trees: - A subclass's own method inherited its base's change: typed() widened a receiver to every base class up to the nearest one the diff changed, so `df.head().T.sum()` was "NDFrame.sum: axis, ... now keyword-only" while pandas 3 still defines `DataFrame.sum` with positional parameters (the diff itself reports the change on NDFrame alone). receiver_map now records the members each class declares (RECEIVERS_SCHEMA 2) and widen() stops at the nearest class that defines the member, or that the diff says changed it. A member the file's own subclass defines is not widened. - The file's own return annotations (`load()`, `self.make()`) were recorded but unreachable in resolve(): they type the chain now. - Values of the file's own classes and methods, of relative imports and of builtins fed the name-only tier (`loc = Local(); loc.swapaxes(0, 1)` was "NDFrame.swapaxes was removed", a warning annotation and exit 3). FileUse.defined records those names; typed() treats their values as the project's own. - A name match changed --fail-on's exit code and produced GitHub annotations although the docs said only that it was left out of the notes. report.counted() leaves it out of both unless --include-name-matches is given, and COMMON_NAMES gains the generic names (`any`, `all`, `sum`, `floor`, `fetch`, `execute`, ...). - The name's uniqueness was computed over the imported packages but matched against the reached ones too: one bare `model.predict` was three "[name match]" uses. type_project types once to learn what each file reaches, then keeps the names unique across imported and reached packages. - A subclass or an isinstance() of a class that became a function was "uses this API": form() takes the use kind and keeps the old form for a reference. `option_context(args=1)` was not refused because `*args` read as a parameter named args, and a `**` in a default disabled the check: apidiff.keyword_parameters tells `*`, `**` and `/` apart. The CHANGELOG bullet is cut to what users notice; the mechanism, the cost of the first scan of a release and the CI behaviour of a name match are in docs/how-it-works.md and the README limitation (four languages).
Owner
Author
|
Review round applied in 8dae090:
Not done: reusing the diff's griffe load for the receiver map (a separate refactor; the map is cached per release after the first scan). |
A verification of the typed receivers found: - The file's own return annotations were keyed by bare name: every method's `make()` and `self.make()` for every class. With `A.make() -> fr.Frame` and `B.make() -> dict`, B's `self.make().swapaxes(0, 1)` was NDFrame.swapaxes (a path match), and a method `A.load() -> fr.Frame` typed a module-level `load()`. A function's return now types a call of its own name only, and `self.m()` the return of the `m` the reading class (or an in-file base) defines; when the classes that read it disagree, or one reaches no `m` in the file, it types nothing. - The guard against widening a member the file defines used every name the file defines, so an unrelated module-level `def predict` hid `self.predict()` in `class Mine(ChatX)`. FileUse.shadowed records the members each class (and its in-file bases) defines over its imported bases, and only those stop the widening. - The name-only tier was mostly noise on ordinary code with pandas imported (`arr.shape`, `obj.to_dict()`, `args.verbose`, `scores.rank()`, `a_set.union()` as polars' Enum.union, `dt.fromisoformat()` as pandas' Timestamp.fromisoformat). Name matches are now hidden from every report (terminal, report.md, the JSON lists, MCP answers), the notes and CI unless --include-name-matches (include_name_matches over MCP) asks for them. The members of the common built-in and standard-library types (from dir() at import) never count by name, COMMON_NAMES gains shape, view, to_dict, to_json, labels, verbose and rank, and a value the file assigns from its own or a builtin value is not unknown: aliases, `store = self.store`, tuple unpacking, `:=`, `except ... as`. - report.counted() filtered per API, so an API with one path match brought its name-only places into --annotate (`def tidy(frame): frame.swapaxes(0, 1)` got a warning). It filters per use. - A bare reference to a class that became a function (`ctx = option_context`) was the old form although valid with the function. Only a subclass, an isinstance()/issubclass() of it (FileUse.type_refs) or an attribute a function does not have is the old form now. docs/how-it-works.md, the README limitation (four languages) and the CHANGELOG bullet say that name matches are hidden unless asked for.
Owner
Author
|
Pushed fixes for what the verification of 8dae090 found:
Docs (how-it-works, README in four languages) and the CHANGELOG bullet are updated. Each fix has a test that fails without it. Gate: ruff, ruff format, mypy and pytest ( |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Audit item 2, the biggest under-report: a changed method was matched only when the file names the receiver's class, so most method calls on values a package returns went unflagged.
DataFrame.applymap was removed,NDFrame.swapaxes was removedandDataFrame.groupby(axis=...)are in the diff, butdf = pd.read_csv(...); df.applymap(str); df.swapaxes(0, 1); df.groupby("key", axis=0)is not flagged.DataFrame.with_row_count,LazyFrame.meltandGroupBy.countare in the diff, butlong = pl.read_csv(...); long.with_row_count(),lf = pl.scan_csv(...); lf.melt(...)anddf.group_by("a").count()are not flagged.BaseChatModel.predict was removedis in the diff, butllm = ChatOpenAI(...); llm.predict(q)is not flagged:ChatOpenAIlives in langchain-openai and inherits across packages, and the file never imports langchain-core.Audit item 8b:
pandas.option_context changed from class to functionlabelledwith pd.option_context("mode.copy_on_write", True):old form, although the call is the same code either way.Change
Item E: receivers typed from the package's own API. No model, nothing run.
apidiff.receiver_map(root): from a griffe-loaded release, what the package's annotations say its values are.classes(every public path of a class -> its canonical path, and a class re-exported from another package -> the path imported),bases(canonical paths; another package's as written),returns(a public function under each of its paths, and a method as<class>.<name>, -> the classes its return annotation names:X,X | None, a union and the@overloadreturns asX|Y, orSelf),attrs(properties and annotated attributes) andmembers(the public names each class declares itself, not inherited). Cached per release and griffe under a newreceiverscache folder (RECEIVERS_SCHEMA = 2). Not part of a diff:DIFF_SCHEMAstays 21 and the fixtures are unchanged (scripts/record_diff_fixtures.py --checkpasses). The first scan of a release loads its sources with griffe a second time for the map (6.6 s for pandas 3.0.6 on this machine, in the main process); every later scan reads the cached map.project.py:scan_filerecords the evidence (FileUse.bound,accesses,assigned,annotated,chain_keywords,defined: import bindings, access chains with()where a call is made, what each name is assigned or annotated with, the file's own functions' returns, the keywords at the end of a chain, and the names the file's own classes, functions and relative imports define).ApiTypesmerges the maps (canonical paths through every package's re-exports, bases, members through the MRO,Self).typed()types each file: a name assigned from a call gets the return annotation's class (pd.read_csv->DataFrame; a class call -> the class), a chain types each step (df.groupby(...).count()->DataFrameGroupBy.count), annotated parameters and variables (string annotations too), the file's own functions' return annotations (df = load(p)afterdef load(p) -> pd.DataFrame,self.make()) andselfin a subclass count, and a class is read with its bases across the project's pinned packages, under every public path, so the member pairs match the paths the diff records. A member is read up to the nearest class that defines it in the pinned release, or that the diff says changed it (selection.changed_owners):df.sum()on aDataFrameisDataFrame.sum, which pandas 3 still defines with positional parameters, notNDFrame.sum(whose parameters became keyword-only; the diff reports it onNDFramealone), andllm.predictisBaseChatModel.predictand not also its base'sBaseLanguageModel.predict.FileUse.reachesrecords the packages a file's values derive from without importing them;Project.code_useincludes such files,Project.importsstays strict ("imported by your code" is unchanged).selection.type_project(project, scans, pypi, store): builds the maps for every package whose pinned tree the scan has (and a cached tree of a package the model knows:PyPI.is_cached, never a download) and returns a copy of the project with its files typed, so a second scan of the same project starts afresh. The matching itself (used_names,uses,form) is unchanged for typed receivers.NAME_ONLY): a member read on a bare name the file shows nothing of (a parameter without an annotation, a loop variable) counts by its name alone when that name is exactly one changed API's across the changed packages the code imports or reaches (a module-level name counts against it) and not inCOMMON_NAMES(get,set,run,create,close,update,count,sum,any,all,floor,fetch,execute, ...). A name assigned a literal, a value of a package the maps do not cover (m = re.match(...)), an instance of the file's own class or of a class it imports relatively, a builtin's value (rows = list()) or what the file's own method returns is not such a value. Tagged[name match]in the terminal (label and place), the Markdown summary, the MCP answer and the JSON ("match": "name_only"), with a legend line. By default it is a hint only: out of the notes, and not counted by--fail-onor--annotate github(report.counted).--include-name-matchesonscanandsynccounts them everywhere (Settings.include_name_matches,ScanResult.name_matches); the block does not keep the choice, which the help says.Item F:
selection.same_call_shape(change): a class <-> function kind change is "uses this API" at a call site, and old form only when the call passes a keyword the new signature does not take (_refused_keywords, fromapidiff.keyword_parameters(new_signature): not*args, not the positional-only ones, and None for**kwargs). A subclass or anisinstance()of it, or any other read without a call (form()now takes the use kind), stays the old form: a function is no type. The note is unchanged.Files outside this item's ownership.
engine.pyandreport.pyget more than one-line hooks, so their owners should look:engine.py(one line inEngine.scanto type the project;Settings.include_name_matches,ScanResult.name_matches;used_changes/diff_notestakename_matches;used_apispassesNAME_ONLYthroughUsedAPI.matchand lists name matches last),report.py(the tag inuse_text, the label, the Markdown form cell, the legend line, "notes ready" counting only what sync writes, andcounted()forfail_reasonandgithub_annotations),cli.py(the flag onscanandsync;receiversinCACHE_NAMESPACES),pypi.py(PyPI.is_cached),apidiff.py(receiver_map,RECEIVERS_SCHEMA,SELF_TYPE,keyword_parameters). PR #80 (file:line locations) editsscan_file's loops andselection._in_file/uses; this change adds separate functions and one keyword line in theFileUse(...)call, so its rebase is small.Before / after (scratch projects, fresh cache,
--cutoff 2025-07-31)data:requirements.txtpins pandas 3.0.6 and polars 2.0.0;main.pyreads a frame withpd.read_csvand callsapplymap,swapaxesandgroupby("key", axis=0)on it, usesoption_context, and with polarspl.DataFrame(...).melt(...),pl.read_csv(...).with_row_count(),pl.DataFrame(...).group_by("a").count()andpl.scan_csv(...).melt(...).Before (main):
After:
lc: pins langchain 1.4.4, langchain-core 1.6.9 and langchain-openai 1.7.0;main.pyimportslangchain.agentsandlangchain_openai.ChatOpenAI, never langchain-core, and callsllm.predict(q)onllm = ChatOpenAI(model="gpt-4o").Before (main):
After:
BaseLanguageModel.predict, which the diff reports on its own, is not a second use:BaseChatModelis the nearest class that lostpredict. The README's sample project (examples/agent-app) still reports the same 2 APIs.Review round
The review of the first version found, on real pandas 3.0.6 and synthetic trees, and this round fixes:
df.head().T.sum()was reported asNDFrame.sum: ... now keyword-onlyalthoughDataFrame.sumstill takes positional arguments (the diff was right; the widening re-attached it). The receiver map now records the members each class declares, and a member is read up to the nearest class that defines it.load()andself.make()never typed a chain). They do now.loc = Local(); loc.swapaxes(0, 1)was "NDFrame.swapaxes was removed", a::warningannotation and exit 3). They are the project's own now.--fail-onand produced GitHub annotations while the docs said only that they were left out of the notes;COMMON_NAMESmissedany,floor,sum,fetchand the like.model.predictwas three[name match]uses.class My(pd.option_context)andisinstance(x, pd.option_context)were "uses this API";option_context(args=1)was not refused (*argswas read as a parameter namedargs), and a**in a default disabled the check.Tests
tests/test_uses.py: synthetic packages in the repo's style (write_tree,FakePyPI,diff_sources), each with an unrelated call in the same project, asserting exact uses:frames(pandas' shape: a value assigned fromread_csv, a constructor through the module, a chain throughgroupby, an inherited removal, a keyword on a typed receiver, a property; a string annotation;selfin a subclass; the file's own return annotations as the only evidence),svc(an overriding subclass:Client.sendandClient.fooof its own, the diff'sBase.sendandBase.foomatched on aBaseonly),core+plugins(langchain's shape: a class read with its bases across packages, in a file that never importscore; the nearest class that lost the member counts, not its base; a name shared by an imported and a reached package counts for neither), the name-only tier with a positive (applymap) and negatives (count, a common name;melt, two APIs'; an own class, a relative import, a builtin and an own method), item F (option_context: a plain call is "uses this API", a refused keyword andargs=the old form, a subclass and anisinstance()the old form, another kind change still the old form) andkeyword_parameters.tests/test_scan_first.py: the[name match]tag in the terminal, the legend, the Markdown form cell, the JSONmatch, the MCP answer, "No notes to write: every use is a name match",fail_reasonandgithub_annotationsleaving a name match out until asked, and--include-name-matchesonscanandsyncthrough the CLI (--fail-on old-formexits 0 on a name match alone, 3 with the flag).tests/test_project.py: the evidencescan_filerecords (definedtoo), andtyped()on hand-made maps (canonical paths through re-exports, ancestors,Self, unions, the nearest-definer and nearest-owner rules, own return annotations,reaches,loose, values of other packages and of the file's own code).Checks:
ruff check src tests scripts,ruff format --check src tests scripts,mypy(strict) andpytest -q -p no:cacheprovider -m "not network"on Python 3.13 and 3.10,pytest -m networkon 3.13 (record_diff_fixtures.py --checkincluded), and the offline suite with griffe 1.14 on 3.10 (the oldest griffe CI runs).