Skip to content

Count lines exactly when the file is smaller than the sample - #119

Open
arpitjain099 wants to merge 1 commit into
dbetchkal:mainfrom
arpitjain099:fix/estimate-line-count-small-files
Open

arpitjain099 wants to merge 1 commit into
dbetchkal:mainfrom
arpitjain099:fix/estimate-line-count-small-files

Conversation

@arpitjain099

Copy link
Copy Markdown
Contributor

estimate_line_count divides by the sample budget rather than by the bytes it actually read, so the scaling factor is wrong whenever the file is smaller than sample_size. In that case the whole file is in the sample and newlines is already the exact count, but it then gets multiplied by file_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 in utils.__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.py with the exact-count case, an estimation case using a reduced sample_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 import utils.helpers, so I wanted to confirm that import chain works before sending it.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
@arpitjain099

Copy link
Copy Markdown
Contributor Author

The failing check here is not from this change. tests/utils/test_db.py breaks with ModuleNotFoundError: No module named 'psycopg' on every open PR since SQLAlchemy 2.1 moved the default postgresql:// driver to psycopg 3, while the workflow installs psycopg2-binary. I opened #120 for that separately.

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 utils.helpers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant