Add an API reference to the documentation - #324
Conversation
The documentation had no API page, which is the largest gap a JOSS reviewer assessing documentation would find. Lists every weighted estimator and weight-handling method on both classes with its signature and a one-line description, taken from the docstrings, plus the estimator conventions that distinguish these from naive weighted equivalents. Registered in both docs/myst.yml and docs/_toc.yml, which are kept in step until the duplicate configs are reconciled.
juaristi22
left a comment
There was a problem hiding this comment.
Two things before this merges, one of them a versioning issue.
1. The fragment releases 1.6.0. api-reference.added.md maps to a minor bump in bump_version.py. I ran it on this branch and it produces 1.5.2 -> 1.6.0. A docs page should be a patch, so rename the fragment to api-reference.changed.md.
2. The page omits the core weighted estimators. I introspected the installed package against docs/api.md. It documents 24 methods but leaves out 23 public names on MicroSeries and 13 on MicroDataFrame, including the ones the README's "Key Features" promises and the closing "estimator conventions" note explains:
MicroSeries:sum,count,mean,median,quantile,var,std,cov,corr,rank,cumsum,groupby,astype,clip,round,repeat,sqrt,values,to_numpy,copy,equalsMicroDataFrame:sum,cov,corr,groupby,merge,reset_index,drop,astype,copy,equals
(scalar_function, vector_function, override_df_functions, catch_series_relapse and get_args_as_micro_series are also public names but read as internals; fine to leave out.) The intro says the page covers weighted estimators and weight-preserving operations, and those two lists are the core of both. The "missing from API" check only runs one way, which is why it passed.
Smaller:
quintile_rank,quartile_rankandpercentile_rankhave empty description cells because they have no docstrings. Better to add the docstrings in the source than to hand-write the cells.set_weightsrendersweights: <built-in function array>because the parameter is annotatednp.arrayinmicroseries.py.np.ndarrayfixes both the page and the annotation.
The conventions note is accurate. I checked the frequency-weight variance and the inverse-CDF median numerically against the docstrings, and the example on the page runs. Agree the _toc.yml / _config.yml reconciliation is separate; both are jupyter-book v1 leftovers, and _config.yml still points at PSLmodels/microdf.
Per review, the page listed the named estimators and the weight helpers but omitted the weighted aggregation methods and the weight-preserving overrides - sum, mean, median, quantile, var, std, cov, corr, rank, groupby, merge, drop and the rest. Those are the core of both things the page says it covers, and the README's feature list promises them. The coverage check only ran one way, from the page to the package, which is why it passed. It is now a test that runs in both directions, so a new public method that is not documented fails. Also adds docstrings to quintile_rank, quartile_rank and percentile_rank rather than hand-writing their description cells, and corrects the set_weights annotation from np.array, a function, to np.ndarray. The changelog fragment becomes .changed so this releases as a patch.
|
All four taken. Thank you — the coverage gap was the important one, and you are right about why it slipped through. Fragment. Renamed to Missing estimators. Added two sections to each class — Weighted aggregation and Weight-preserving operations — covering everything on your lists. The page goes from 26 documented methods to 59. The one-way check. Made it a test rather than a script I run once: Docstrings and the annotation. Added docstrings to Where a pandas docstring described the unweighted behaviour and said nothing about weights, I wrote the description cell rather than inheriting a misleading first line; Full suite passes, 837 tests. Noting your |
|
@juaristi22 all four points are addressed in Re-review when you have a moment. The two-way check is the part worth a second look — I scoped it to names each class defines or overrides itself, on the reasoning that anything inherited unchanged from pandas is pandas' to document, and excluded the five internals you listed. If you would rather it covered inherited names too, that is a different and much longer page, and I would rather agree the scope than guess it. All 8 checks green, including the documentation build. |
juaristi22
left a comment
There was a problem hiding this comment.
Second pass on 4eb1082. The fragment, the coverage test and the page structure check out; three of the new descriptions do not.
What holds up. bump_version.py on this branch gives 1.5.2 -> 1.5.3 (patch). The two-way test is real: adding an undocumented method to MicroSeries and adding a bogus row to api.md each fail the right assertion. Full suite passes locally (837). I built the page with mystmd: all eleven tables render, and the escaped pipe in the DataFrame cov row comes out as one cell with a literal pipe. On scope: own and overridden names only, with the five internals excluded, is the right scope. Inherited pandas names are pandas' to document.
Blocking.
-
MicroDataFrame.covandcorrare not weighted, but the rows say they are. The overrides call pandas'cov/corron the plain frame, and the source comment says as much ("Column summaries have no observation weights"). Numerically, withx=[1,2,3,4],y=[1,4,2,8],w=[1,1,1,5]:MicroDataFrame.cov()gives 3.1667, the same as unweighted pandas, while the frequency-replicated sample gives 3.1786, which is whatm.x.cov(m.y)returns.corr()is 0.792 against 0.896 replicated. The rows sit under "Weighted aggregation" and read "Pairwise frequency-weighted covariance/correlation of the columns." Either say plainly that these return unweighted pandas results, or move them out of that section. Making them weighted is a code change, filed as #327. -
equalscompares weights on both classes. Both implementations returnequal_values and equal_weights. The rows say "weights are not part of the comparison", which is the opposite. -
The
set_weightsrow still reads<built-in function array>. The annotation is fixed in source but the page row was not regenerated, so the rendered page still shows the artifact. The newquantilerow has the same artifact fromq: np.arrayatmicroseries.py:596, which is unchanged.
Smaller.
valuesis a property, so the()signature cell is misleading. Mark it as an attribute or leave the cell blank.- The test ships in both the sdist and the wheel (
include = ["microdf*"]picks upmicrodf.tests), butdocs/ships in neither, and with the file absent the test fails rather than skips (two failures). A module-levelpytest.skipwhenDOCSis missing covers anyone running the packaged tests. inspectis imported but unused, andpublic_methodshas an unusedbaseparameter. Lint only runs the formatters, so nothing catches these.- The comments cite
9c0a9cd, which is not on the remote; the head is4eb1082.
MicroDataFrame.cov and corr do not use the weights - they call pandas on the plain frame - but the rows described them as frequency-weighted and sat under Weighted aggregation. Verified: with w=[1,1,1,5] the frame gives 3.1667, the unweighted pandas value, while the replicated sample and MicroSeries.cov both give 3.1786. They now carry a warning pointing at #327 and say plainly that they are unweighted. equals compares weights on both classes - both return equal_values and equal_weights - and the rows claimed the opposite. The set_weights row still rendered '<built-in function array>' because the page was not regenerated after the annotation fix, and quantile had the same artifact from an unfixed 'q: np.array'. Both annotations are now np.ndarray in source, in microseries.py and microdataframe.py, and the rows regenerated. Also marks values as an attribute rather than showing a call signature, skips the coverage test when docs/ is absent so the packaged tests pass, and drops an unused import and parameter.
|
All three blocking points were real errors of mine, and I reproduced each before fixing. Head is now 1. 2. 3. The rendering artifacts. You are right that I fixed the annotation and never regenerated the page. Smaller. Thank you for the second pass — three hand-written description cells asserting the opposite of what the code does is exactly the failure mode a generated page invites, and I would not have caught the |
Audited every description on the page that was written by hand rather than taken from a docstring, since that is where all three errors found in review were. cumsum was the remaining one. It returns a plain pandas Series and warns that the weights have been applied and cannot be reused, so it is the one method in the weight-preserving section that does not preserve them. The page now says so. The rest hold: clip, round, sqrt, copy and astype carry weights through; repeat repeats them alongside the values; merge, reset_index and drop keep them aligned, and drop(index=) drops the matching weights; groupby returns the MicroSeriesGroupBy and MicroDataFrameGroupBy wrappers. Those checks are now a test rather than something I ran once, covering the claims no docstring enforces: the unweighted frame cov and corr, the weighted MicroSeries cov, equals comparing weights, cumsum dropping them, and repeat carrying them.
|
Audited every remaining description on the page that I wrote by hand rather than took from a docstring, on the grounds that all three errors you found were in exactly those cells. One more was wrong: The rest hold, checked by running them: And they are now a test rather than something I ran once. 838 tests pass, formatter clean. |
juaristi22
left a comment
There was a problem hiding this comment.
Third pass on e6df79a. Every round-2 item is fixed and holds up; one item from round 1 is still open.
Verified. The DataFrame cov and corr rows sit outside the weighted table under a warning box that says unweighted and links #327. The equals rows now say weights are compared. The set_weights and quantile rows are regenerated and show numpy.ndarray, and no np.array annotation remains in the source. values renders as an attribute. The coverage test skips when docs/api.md is absent (3 skipped with the file moved aside), and ruff check --select F is clean on it. The cumsum correction is right. 838 pass locally, the page builds with mystmd, and the bump is a patch.
Still open: the three rank rows are empty. quintile_rank, quartile_rank and percentile_rank got docstrings in round 1, but their description cells on the page were never regenerated and are still blank:
| `quintile_rank` | `() -> 'MicroSeries'` | |
| `quartile_rank` | `() -> 'MicroSeries'` | |
| `percentile_rank` | `() -> 'MicroSeries'` | |
Same miss as the set_weights row in round 2: the source changed, the page did not. One regeneration fixes all three.
Coordination with #330. test_documented_weight_behaviour_holds pins the frame's cov and corr to unweighted pandas. I trial-merged this branch with #330: the merge is textually clean, but that assertion fails on the merged tree (3.1786 against 3.1667), as you predicted. Whichever lands second has to flip the assertion and rewrite the two rows and the warning box. The clean order is this PR first, since it is accurate about main today; I will then rebase #330 and update the page and the test there.
The three rank rows were still blank: they got docstrings in the first round but the page was never regenerated, the same miss as set_weights in the second. Patching individual rows by hand is what kept producing this, so the rows are now regenerated from the live signatures and docstrings, and two tests hold the page to the code. test_no_row_is_missing_its_description fails on any blank description cell. test_signatures_match_the_live_ones compares every rendered signature against inspect.signature, attributing each row to the class whose heading it falls under, since several names exist on both. The second test immediately found a real error: the MicroDataFrame set_weights row carried the MicroSeries signature, because the earlier regex fix matched both rows. Regenerated.
|
Fixed, and I have stopped the class of error rather than just this instance. The three rank rows. Regenerated from the docstrings — quintile, quartile and percentile now read "Calculate weighted quintile/quartile/percentile ranks". You are right that it is the same miss as So the page is now held to the code by two tests.
The second test immediately caught a real error I had introduced and you had not seen yet: the 840 pass, formatter clean. On merge order — agreed, and your way round rather than mine. You are right that this branch is accurate about |
juaristi22
left a comment
There was a problem hiding this comment.
Fourth pass on 552dc4c. The rank rows and the MicroDataFrame.set_weights row are right, and the blank-description test is sound. On Python 3.9 with pandas 2.3.3 the full suite passes at 840, which is the environment behind "840 pass". CI is red, though, and the new signature test cannot go green across the matrix as written.
Blocking: test_signatures_match_the_live_ones compares annotation reprs as text, and they differ between pandas 2 and pandas 3. The page was rendered under pandas 2, where a Series annotation renders as pandas.core.series.Series; pandas 3 renders the same annotation as pandas.Series. Pandas 3 requires Python 3.11 or newer, so the 3.9 and 3.10 jobs resolve pandas 2 and pass, while 3.11, 3.12 and 3.13 resolve pandas 3 and fail. The 3.13 job failed on exactly seven rows, all this one cause: quantile, cov, corr, rank, cumsum and weight on MicroSeries, and sum on MicroDataFrame. The other four jobs were cancelled after it. Regenerating the page under pandas 3 would move the failure to 3.9 and 3.10 instead.
Compare structure rather than text: parameter names, kinds and defaults from inspect.signature are stable across versions. Leave the annotation text unchecked, or normalise it on both sides before comparing. Forward note: on Python 3.14, not yet in the matrix, Optional[X] also renders as X | None, which takes the mismatch count to 23, so text comparison only gets worse.
Root cause of the four regeneration misses. The generator that produces the rows lives outside the repo and is run by hand. Committing it, for example as docs/build_api.py, makes regeneration one command, and lets the test call the same renderer, which is where any normalisation belongs.
Merge order as agreed: this PR first once green, then I rebase #330 and update the two rows, the warning box and the cov/corr assertion there.
|
Merge order flipped after Vahid's review on #330: #330 lands first. This PR then drops the warning box, relabels |
The script that produced docs/api.md was run by hand and lived outside the repo, which is why the source outran the page four times in review. It is now docs/build_api.py: the prose is held verbatim, the tables are generated from the live classes, and the hand-written descriptions that are not docstrings (cumsum, merge, reset_index, drop, equals, copy, groupby, clip, round, repeat, sqrt, and the two unweighted cov/corr rows) are an explicit override table so regeneration cannot lose them. str(inspect.signature(...)) was never safe to compare as text: pandas 2 renders a Series annotation as pandas.core.series.Series and pandas 3 renders it as pandas.Series, so a page generated under one fails CI on the other's jobs, and from Python 3.14 Optional[X] reprs as X | None. microdf/_docs.py renders signatures from parameter names, kinds and defaults, strips module qualifiers and puts every union into one form. The generator and the test both import it, so the page cannot disagree with the test, and the same committed page passes under pandas 2.3 and 3.0 and on Python 3.9 through 3.14.
#330 merged, so MicroDataFrame.cov and corr now use the weights. Drops the warning box and folds both back into the weighted aggregation table, with the descriptions restored to frequency-weighted, and flips the assertion in test_documented_weight_behaviour_holds to the replicated sample. It also now asserts each frame cell equals the corresponding MicroSeries value, which is the property #330 introduced. The CI Lint failure was docformatter rather than ruff - make lint runs both, and the new _docs.py needed its docstrings rewrapped. Merges main in, so the branch carries #330 and #331.
|
@juaristi22 green across the whole matrix now, The version-fragile comparison is gone, and the generator is committed. Verified the way you framed the problem — same committed page, both pandas majors: Also run on 3.9, 3.10 and 3.14, the last being where your 23-row Your root-cause diagnosis was the valuable part. The generator living outside the repo and being run by hand is exactly why the page fell behind the source four separate times, and none of the individual fixes would have stopped a fifth. #330 handover done. The warning box is gone, On the red build: 852 pass. Over to you for a fifth look — sorry it took five. |
juaristi22
left a comment
There was a problem hiding this comment.
Fifth pass on a37aab0. Everything from the earlier rounds is closed, by construction rather than by hand.
Version-stable comparison. One renderer in microdf/_docs.py builds each signature from parameter names, kinds and defaults, strips module qualifiers so pandas 2 and pandas 3 render alike, and collapses the union spellings, including the Python 3.14 form. The generator and the test both import it, so the page and the test cannot disagree. CI is green across the matrix; locally 852 pass on Python 3.9 with pandas 2.3.3 and on Python 3.14 with pandas 3.0.6.
Generator committed. I ran docs/build_api.py under both pandas majors and got a byte-identical page each time. Descriptions come from docstrings, the curated cells sit in one overrides table, and the script refuses to write a row with no description. With the blank-cell test, the miss that recurred four times cannot recur.
#330 handover. Warning box gone, cov and corr back in the weighted table as frequency-weighted, and test_documented_weight_behaviour_holds checks the replicated sample and that each frame cell equals its MicroSeries value. The branch contains main, so no rebase is needed.
Lint is clean on the new files. One observation, not a request: the renderer lives inside the package and ships in the wheel; that is right given the tests ship and import it, and nothing imports it at runtime.
Approving. Thanks for staying with it for five rounds; the page is now held to the code in both directions.
Merges main, so the branch carries the weighted MicroDataFrame cov and corr from #330 and the API reference from #324. Without it a reviewer checking out this branch would see behaviour the paper does not describe. Corrects the replication paragraph: the ACS and CPS ASEC use successive-difference replication, which is the 4/R scale the paper quotes. Fay's variant of BRR is a different scheme with a 1/(R(1-k)^2) scale, and replication.py already keeps the two apart. Refreshes the state of the field. R's convey is the closest existing equivalent to microdf's estimator set and was absent; the table said R survey had limited inequality measures, which is true of survey alone and misleading once convey exists. samplics is now archived in favour of svy, and the paper said neither. Adds what DescrStatsW does cover. Adds a paragraph placing this work against the published policyengine paper, since a reviewer will otherwise ask why the dependency is not covered there.
The landing page said function documentation would arrive 'in the future', which #324 has since added, so the first page of the docs told a reader the API reference does not exist. It now describes what the package does, installs it, and links to both pages. examples.md opened with 'See these rendered Jupyter notebooks' followed by no links. It now links the one notebook there is. The roadmap claimed graphs and Tax-Calculator helpers, neither of which is in the package - grep finds no taxcalc reference and no plotting code - and listed replicate-weight standard errors as future work, which shipped in #320 and is the paper's headline feature. Replaced with what the package does and three things it does not do yet.
The documentation has no API page. For a JOSS submission that is the largest documentation gap — a reviewer is asked directly whether the functionality is documented.
docs/api.mdlists every weighted estimator and weight-handling method onMicroSeriesandMicroDataFrame, with signature and a one-line description taken from the docstrings, grouped as:gini, the top/bottom share family,t10_b50), ranking (decile_rankand friends), variance from replicate weights, and weight handlingpoverty_rate,poverty_gap, the deep and squared variants), and weight handlingreplicate_variance,replicate_standard_error26 methods in total. I checked every documented name against the installed package, so the page cannot drift from the API without the check failing:
The page closes with the estimator conventions a user needs in order to interpret the numbers — inverse-CDF quantiles checkable against
survey::svyquantile, frequency-weight variance, and proportional splitting at top-share cutoffs. These are the decisions the paper argues are the package's substance, and they were documented nowhere a user would find them.Registered in both
docs/myst.ymlanddocs/_toc.yml. The two configs duplicate each other and should be reconciled, but that is a separate change; until then, leaving one out would silently drop the page from whichever builder is used.I could not build the docs locally — no Node — so the documentation job on this PR is the check that matters.