Repository navigation
Count lines exactly when the file is smaller than the sample - #119
Open
arpitjain099 wants to merge 1 commit into
Open
arpitjain099 wants to merge 1 commit into
arpitjain099 wants to merge 1 commit into
Conversation
Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
Contributor
Author
|
The failing check here is not from this change. The three tests added here pass; I ran them in a container matching the workflow, including the GDAL build, since this is the first test to import |
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.
estimate_line_countdivides by the sample budget rather than by the bytes it actually read, so the scaling factor is wrong whenever the file is smaller thansample_size. In that case the whole file is in the sample andnewlinesis already the exact count, but it then gets multiplied byfile_size / 1MB, which is less than one. Measured on the current code: a 1000-line 30KB file returns 29, and a 10-line file returns 0.The private copy of this helper on
models.py(_estimate_line_count, used for the FAA data progress bar) already has the guard, so this change just brings the exported one in line with its twin rather than inventing behaviour. Nothing in the repo calls the exported version today, so this is an API-surface fix rather than a live failure, but it is inutils.__all__and returns a plausible-looking number instead of an error, which seems worth correcting.Verified by running the function both ways: files under the sample size now return the exact count, and files above it are unchanged (a 3.88MB file estimates 80626 lines either way, as does a 1.56MB one). Added
tests/utils/test_helpers.pywith the exact-count case, an estimation case using a reducedsample_size, and the no-newline case. I ran these in a container matching the CI workflow (GDAL 3.10.3 plus the full dependency list), where all three pass; this is the first test to importutils.helpers, so I wanted to confirm that import chain works before sending it.