Skip to content

added project argument to predict_tile for geo-coordinates#1250

Open
musaqlain wants to merge 1 commit into
weecology:mainfrom
musaqlain:feature-predict-tile-projection
Open

added project argument to predict_tile for geo-coordinates#1250
musaqlain wants to merge 1 commit into
weecology:mainfrom
musaqlain:feature-predict-tile-projection

Conversation

@musaqlain
Copy link
Copy Markdown
Contributor

@musaqlain musaqlain commented Dec 27, 2025

addresses Issue #608. Previously, predict_tile returned pixel coordinates even if the input was a georeferenced raster. This PR adds an optional project=False argument to allow users to request map coordinates directly.

  • updated predict_tile in src/deepforest/main.py to accept project (bool).
  • if project=True, the results are converted to a GeoDataFrame using utilities.image_to_geo_coordinates.
  • added test_predict_tile_projected to tests/test_main.py to verify that setting project=True returns a GeoDataFrame with valid CRS and map coordinates (UTM).

Closes #608

AI-Assisted Development

I utilized AI for sanity checking code logic and conducting an assisted review to identify potential missing resets and schema constraints.

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

Copy link
Copy Markdown
Member

@ethanwhite ethanwhite left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry this got lost in the queue @musaqlain. Everything looks really good. I've requested a couple of small changes to the docstring.

My one bigger question is what happens if someone sets project=True and passes a path to a non-georeferenced file. It would be good to make sure that we fail informatively in this case, which might happen automatically or might require a new check either in the new if project: block or in image_to_geo_coordinates

Comment thread src/deepforest/main.py Outdated
Comment thread src/deepforest/main.py Outdated
@ethanwhite ethanwhite added the Awaiting author contribution Waiting on the issue author to do something before proceeding label Apr 17, 2026
@musaqlain
Copy link
Copy Markdown
Contributor Author

It would be good to make sure that we fail informatively in this case

Thanks for pointing this out! I have added a check in the if project: block that opens the image and verifies if src.crs is None.
If so, it now raises an informative ValueError and will be advising the user to either use a georeferenced raster or set project=False. Thank you and you can review this too. 👍

@musaqlain musaqlain requested a review from ethanwhite May 20, 2026 18:36
@github-actions github-actions Bot removed the Awaiting author contribution Waiting on the issue author to do something before proceeding label May 20, 2026
@musaqlain musaqlain force-pushed the feature-predict-tile-projection branch from 2461cad to 3ed720f Compare May 20, 2026 18:42
@musaqlain musaqlain force-pushed the feature-predict-tile-projection branch from 3ed720f to be8794b Compare May 20, 2026 18:53
@codecov
Copy link
Copy Markdown

codecov Bot commented May 20, 2026

Codecov Report

❌ Patch coverage is 18.18182% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.40%. Comparing base (f3bd776) to head (be8794b).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
src/deepforest/main.py 18.18% 9 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1250      +/-   ##
==========================================
- Coverage   86.96%   86.40%   -0.56%     
==========================================
  Files          26       26              
  Lines        3712     3744      +32     
==========================================
+ Hits         3228     3235       +7     
- Misses        484      509      +25     
Flag Coverage Δ
unittests 86.40% <18.18%> (-0.56%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 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.

Copy link
Copy Markdown
Member

@ethanwhite ethanwhite left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks really good. One additional docstring change suggestion and a question about whether there is some duplicate code being added

Comment thread src/deepforest/main.py
Comment on lines -598 to +601
pd.DataFrame or tuple: Predictions dataframe or (predictions, crops) tuple
pd.DataFrame, geopandas.GeoDataFrame, or tuple: Predictions dataframe, geopandas.GeoDataFrame, or (predictions, crops) tuple.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably a little more cleanly: "pd.DataFrame, geopandas.GeoDataFrame, or tuple: Predictions dataframe with or without geometry or (predictions, crops) tuple.

Comment thread src/deepforest/main.py
Comment on lines +758 to +759
if root_dir is None:
root_dir = os.path.dirname(paths[0])
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the code starting on Line 723 already cover this?

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.

Outputs should attach geometry if available

2 participants