Repository navigation
Handle bounds for discrete and zero-inflated densities - #37
Merged
Merged
Conversation
tnagler
changed the title
This was referenced Aug 26, 2026
tnagler
force-pushed
the
discrete-boundary-transforms
branch
from
August 26, 2026 12:25
9f9a603 to
0c21aae
Compare
tnagler
pushed a commit
that referenced
this pull request
Aug 26, 2026
These `//!` comments are lifted verbatim into pyvinecopulib's Python API documentation, so whatever is written here is what a Python user reads on the rendered site. Several of them had drifted into describing the implementation, and a few had become wrong. Factually wrong: - `get_actual_grid_size` blamed the difference on "boundary-snapping in `finalize_grid()`". `finalize_grid` only reverses now, and the count is always exactly one more than requested, never "slightly" different. It also cross-referenced a private method. - `quantile` documented its domain as `(0, 1)` while the guard accepts `[0, 1]` and the tests added in #37 rely on the closed interval. The exception message said the same wrong thing; both now say `[0, 1]`. - `get_edf` described the sum of per-observation influences, which stopped being the whole story once boundary repair could fuse them, and never mentioned the zero-inflated `+1`. - `as_enum` accepts `"zero_inflated"` as well as `"zero-inflated"`, but neither the `@param type` text nor the error message listed it. - Both grid constructors document `@param prob0` while declaring the parameter as `prob0_`, so a doc tool sees one undocumented parameter and one `@param` for a parameter that does not exist. The trailing underscore was the bug; the declarations now match the definitions. Under-documented, in ways that matter downstream: - What `xmin` / `xmax` *mean* now differs by variable type -- integer support for a discrete variable, continuous-component-only for a zero-inflated one -- and that was stated only in the overview pages. It is on the constructors and on `get_xmin` / `get_xmax` now, since that is where a caller looks. - `get_grid_points` and `get_values` did not say that a discrete grid runs half a unit past each bound, nor that its values are the jitter density rather than probabilities. - `fit` listed no exceptions at all, though it can now raise five distinct ones, and did not say that NaNs are dropped or that weights are rescaled. - `set_xmin_xmax` did not mention that it throws after a fit, or on a non-integer discrete bound. - `str()` had no comment, so the Python `__repr__` inherited an empty one. - `VarType` and its three members had none either. Rewritten away from mechanism: the class summary, `get_bandwidth`, `get_boundary_repair` and `@param boundary_repair` led with "bulk estimator", "boundary experts", "endpoint classifier" and "fusion weights" -- terms defined nowhere a Python reader can reach. They now say what the thing is and what it means for the caller, and point at the overview pages for the estimator itself. `@param boundary_repair` also states its default and that it is eligibility rather than a guarantee. Smaller: `@param check_fitted` said "logical", which is R's word, and read backwards; `@param seeds` never said what an empty vector does; `@param bandwidth`'s continuation line was unindented, which truncates the description in a numpydoc converter that uses indentation to find one; and `@param type` carried a double space that survives into the rendered text. No behavior changes, with two deliberate exceptions that are corrections to what the code says about itself: the quantile exception message, and the `as_enum` message.
tvatter
added a commit
that referenced
this pull request
Aug 27, 2026
These `//!` comments are lifted verbatim into pyvinecopulib's Python API documentation, so whatever is written here is what a Python user reads on the rendered site. Several of them had drifted into describing the implementation, and a few had become wrong. Factually wrong: - `get_actual_grid_size` blamed the difference on "boundary-snapping in `finalize_grid()`". `finalize_grid` only reverses now, and the count is always exactly one more than requested, never "slightly" different. It also cross-referenced a private method. - `quantile` documented its domain as `(0, 1)` while the guard accepts `[0, 1]` and the tests added in #37 rely on the closed interval. The exception message said the same wrong thing; both now say `[0, 1]`. - `get_edf` described the sum of per-observation influences, which stopped being the whole story once boundary repair could fuse them, and never mentioned the zero-inflated `+1`. - `as_enum` accepts `"zero_inflated"` as well as `"zero-inflated"`, but neither the `@param type` text nor the error message listed it. - Both grid constructors document `@param prob0` while declaring the parameter as `prob0_`, so a doc tool sees one undocumented parameter and one `@param` for a parameter that does not exist. The trailing underscore was the bug; the declarations now match the definitions. Under-documented, in ways that matter downstream: - What `xmin` / `xmax` *mean* now differs by variable type -- integer support for a discrete variable, continuous-component-only for a zero-inflated one -- and that was stated only in the overview pages. It is on the constructors and on `get_xmin` / `get_xmax` now, since that is where a caller looks. - `get_grid_points` and `get_values` did not say that a discrete grid runs half a unit past each bound, nor that its values are the jitter density rather than probabilities. - `fit` listed no exceptions at all, though it can now raise five distinct ones, and did not say that NaNs are dropped or that weights are rescaled. - `set_xmin_xmax` did not mention that it throws after a fit, or on a non-integer discrete bound. - `str()` had no comment, so the Python `__repr__` inherited an empty one. - `VarType` and its three members had none either. Rewritten away from mechanism: the class summary, `get_bandwidth`, `get_boundary_repair` and `@param boundary_repair` led with "bulk estimator", "boundary experts", "endpoint classifier" and "fusion weights" -- terms defined nowhere a Python reader can reach. They now say what the thing is and what it means for the caller, and point at the overview pages for the estimator itself. `@param boundary_repair` also states its default and that it is eligibility rather than a guarantee. Smaller: `@param check_fitted` said "logical", which is R's word, and read backwards; `@param seeds` never said what an empty vector does; `@param bandwidth`'s continuation line was unindented, which truncates the description in a numpydoc converter that uses indentation to find one; and `@param type` carried a double space that survives into the rendered text. No behavior changes, with two deliberate exceptions that are corrections to what the code says about itself: the quantile exception message, and the `as_enum` message.
tvatter
force-pushed
the
discrete-boundary-transforms
branch
from
August 27, 2026 07:12
0c21aae to
d07006c
Compare
tvatter
added a commit
that referenced
this pull request
Aug 27, 2026
These `//!` comments are lifted verbatim into pyvinecopulib's Python API documentation, so whatever is written here is what a Python user reads on the rendered site. Several of them had drifted into describing the implementation, and a few had become wrong. Factually wrong: - `get_actual_grid_size` blamed the difference on "boundary-snapping in `finalize_grid()`". `finalize_grid` only reverses now, and the count is always exactly one more than requested, never "slightly" different. It also cross-referenced a private method. - `quantile` documented its domain as `(0, 1)` while the guard accepts `[0, 1]` and the tests added in #37 rely on the closed interval. The exception message said the same wrong thing; both now say `[0, 1]`. - `get_edf` described the sum of per-observation influences, which stopped being the whole story once boundary repair could fuse them, and never mentioned the zero-inflated `+1`. - `as_enum` accepts `"zero_inflated"` as well as `"zero-inflated"`, but neither the `@param type` text nor the error message listed it. - Both grid constructors document `@param prob0` while declaring the parameter as `prob0_`, so a doc tool sees one undocumented parameter and one `@param` for a parameter that does not exist. The trailing underscore was the bug; the declarations now match the definitions. Under-documented, in ways that matter downstream: - What `xmin` / `xmax` *mean* now differs by variable type -- integer support for a discrete variable, continuous-component-only for a zero-inflated one -- and that was stated only in the overview pages. It is on the constructors and on `get_xmin` / `get_xmax` now, since that is where a caller looks. - `get_grid_points` and `get_values` did not say that a discrete grid runs half a unit past each bound, nor that its values are the jitter density rather than probabilities. - `fit` listed no exceptions at all, though it can now raise five distinct ones, and did not say that NaNs are dropped or that weights are rescaled. - `set_xmin_xmax` did not mention that it throws after a fit, or on a non-integer discrete bound. - `str()` had no comment, so the Python `__repr__` inherited an empty one. - `VarType` and its three members had none either. Rewritten away from mechanism: the class summary, `get_bandwidth`, `get_boundary_repair` and `@param boundary_repair` led with "bulk estimator", "boundary experts", "endpoint classifier" and "fusion weights" -- terms defined nowhere a Python reader can reach. They now say what the thing is and what it means for the caller, and point at the overview pages for the estimator itself. `@param boundary_repair` also states its default and that it is eligibility rather than a guarantee. Smaller: `@param check_fitted` said "logical", which is R's word, and read backwards; `@param seeds` never said what an empty vector does; `@param bandwidth`'s continuation line was unindented, which truncates the description in a numpydoc converter that uses indentation to find one; and `@param type` carried a double space that survives into the rendered text. No behavior changes, with two deliberate exceptions that are corrections to what the code says about itself: the quantile exception message, and the `as_enum` message.
tvatter
added a commit
that referenced
this pull request
Aug 27, 2026
These `//!` comments are lifted verbatim into pyvinecopulib's Python API documentation, so whatever is written here is what a Python user reads on the rendered site. Several of them had drifted into describing the implementation, and a few had become wrong. Factually wrong: - `get_actual_grid_size` blamed the difference on "boundary-snapping in `finalize_grid()`". `finalize_grid` only reverses now, and the count is always exactly one more than requested, never "slightly" different. It also cross-referenced a private method. - `quantile` documented its domain as `(0, 1)` while the guard accepts `[0, 1]` and the tests added in #37 rely on the closed interval. The exception message said the same wrong thing; both now say `[0, 1]`. - `get_edf` described the sum of per-observation influences, which stopped being the whole story once boundary repair could fuse them, and never mentioned the zero-inflated `+1`. - `as_enum` accepts `"zero_inflated"` as well as `"zero-inflated"`, but neither the `@param type` text nor the error message listed it. - Both grid constructors document `@param prob0` while declaring the parameter as `prob0_`, so a doc tool sees one undocumented parameter and one `@param` for a parameter that does not exist. The trailing underscore was the bug; the declarations now match the definitions. Under-documented, in ways that matter downstream: - What `xmin` / `xmax` *mean* now differs by variable type -- integer support for a discrete variable, continuous-component-only for a zero-inflated one -- and that was stated only in the overview pages. It is on the constructors and on `get_xmin` / `get_xmax` now, since that is where a caller looks. - `get_grid_points` and `get_values` did not say that a discrete grid runs half a unit past each bound, nor that its values are the jitter density rather than probabilities. - `fit` listed no exceptions at all, though it can now raise five distinct ones, and did not say that NaNs are dropped or that weights are rescaled. - `set_xmin_xmax` did not mention that it throws after a fit, or on a non-integer discrete bound. - `str()` had no comment, so the Python `__repr__` inherited an empty one. - `VarType` and its three members had none either. Rewritten away from mechanism: the class summary, `get_bandwidth`, `get_boundary_repair` and `@param boundary_repair` led with "bulk estimator", "boundary experts", "endpoint classifier" and "fusion weights" -- terms defined nowhere a Python reader can reach. They now say what the thing is and what it means for the caller, and point at the overview pages for the estimator itself. `@param boundary_repair` also states its default and that it is eligibility rather than a guarantee. Smaller: `@param check_fitted` said "logical", which is R's word, and read backwards; `@param seeds` never said what an empty vector does; `@param bandwidth`'s continuation line was unindented, which truncates the description in a numpydoc converter that uses indentation to find one; and `@param type` carried a double space that survives into the rendered text. No behavior changes, with two deliberate exceptions that are corrections to what the code says about itself: the quantile exception message, and the `as_enum` message.
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
2 participants
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.
Summary
Testing
This PR is stacked on #36 and should be rebased or retargeted to main after #36 merges.
Closes #35