Skip to content

feat: allow predict_file to accept pandas DataFrame instead of only C… - #1409

Open
proofbyhuman wants to merge 3 commits into
weecology:mainfrom
proofbyhuman:fix-predict-file-dataframe
Open

feat: allow predict_file to accept pandas DataFrame instead of only C…#1409
proofbyhuman wants to merge 3 commits into
weecology:mainfrom
proofbyhuman:fix-predict-file-dataframe

Conversation

@proofbyhuman

Copy link
Copy Markdown

Description

This PR resolves issue #797 by allowing the predict_file method to accept a pandas DataFrame directly, in addition to CSV file paths.

Changes Made:

  1. src/deepforest/main.py: Renamed the primary argument from csv_file to input_file. Added csv_file as an optional deprecated argument with a DeprecationWarning to maintain backward compatibility.
  2. src/deepforest/datasets/prediction.py: Updated the FromCSVFile class __init__ type hint to accept str | pd.DataFrame, removing the previous restriction that forced the input to be a string.

Why this works:
The downstream function utilities.read_file already natively supports both str and pd.DataFrame inputs. This change simply removes the unnecessary type restriction at the entry point, fulfilling the feature request without altering the core processing logic.

Testing:
Local environment constraints prevented running the full test suite locally, but the changes are minimal and strictly type-hint/deprecation additions. GitHub Actions will verify that existing tests continue to pass.

Related Issue(s)

Closes #797

AI-Assisted Development

  • I used AI tools (e.g., GitHub Copilot, ChatGPT, etc.) in developing this PR
  • I understand all the code I'm submitting
  • I have reviewed and validated all AI-generated code

AI tools used (if applicable):

  • Qwen (AI assistant) was used to guide the step-by-step learning process, explain open-source contribution practices, and draft the code modifications. All code was manually reviewed, understood, and validated by the contributor before committing.

…l sites

Addresses review feedback on weecology#1409.

- FromCSVFile.prepare_items called read_file without root_dir. That worked
  while csv_file was always a path, because read_file infers the root from
  the file's directory, but this PR also accepts DataFrames, where there is
  no path to infer from. predict_file(input_file=df, root_dir=...) then
  failed with "root_dir argument not specified and input is a dataframe".
  Forward root_dir so the argument is honoured.

- Update the remaining predict_file call sites to input_file, including
  scripts/predict.py and the user guide. Two of them passed the file
  positionally, so they silently rebound to input_file without warning.
  test_predict_file_deprecated_csv_file keeps csv_file on purpose, since
  it asserts the DeprecationWarning is raised.

- root_dir docstring promised a fallback to "image_dir" in config, but no
  such key exists. Describe what actually happens instead.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@proofbyhuman

Copy link
Copy Markdown
Author

Both points addressed.

Call sites. All 13 are updated — the 11 keyword ones (src/deepforest/scripts/predict.py, tests/test_evaluate.py, tests/test_IoU.py, tests/test_main.py, tests/profile_evaluate.py, tests/profile_predict_file.py, docs/user_guide/12_evaluation.md) plus the two positional ones in tests/test_main.py and docs/user_guide/16_prediction.md, which I switched to the explicit input_file= keyword.

root_dir docstring. It no longer promises a config fallback that isn't implemented. It now describes what the code actually does: if root_dir is None it comes from the directory of input_file, which therefore has to be a path rather than a DataFrame. I didn't implement the config fallback, since that would be new behaviour rather than a fix — happy to add it if you'd prefer that instead.

While updating the call sites I hit the bug that was actually breaking the DataFrame path: FromCSVFile.prepare_items wasn't forwarding root_dir to read_file, so a DataFrame input failed. Fixed in the same commit.

Local runs on this branch:

$ uv run pytest tests/test_IoU.py tests/test_evaluate.py tests/test_datasets_prediction.py
27 passed in 449.25s
$ uv run pytest tests/test_main.py -k predict_file
3 passed
$ uv run pytest tests/test_cli_predict.py
7 passed in 648.05s

@codecov

codecov Bot commented Aug 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 86.67%. Comparing base (d2851ec) to head (fcbc893).
⚠️ Report is 14 commits behind head on main.

Files with missing lines Patch % Lines
src/deepforest/main.py 80.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1409      +/-   ##
==========================================
+ Coverage   86.61%   86.67%   +0.05%     
==========================================
  Files          26       28       +2     
  Lines        3736     4075     +339     
==========================================
+ Hits         3236     3532     +296     
- Misses        500      543      +43     
Flag Coverage Δ
unittests 86.67% <88.88%> (+0.05%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

main.deepforest.predict_file should be able to take in a dataframe, not just a csv file.

1 participant