feat(helpers): accept & auto-convert ee.Geometry in fit_geometry

[abidanBrito]

Closes #310

Summary

fit_geometry previously accepted only shapely geometries. Passing an ee.Geometry produced confusing downstream errors with no hint at the expected conversion. This PR adds guarded auto-conversion for Earth Engine-like geometry inputs, with a clear, actionable TypeError when conversion isn't possible.

Changes

• New _coerce_to_shapely_geometry helper:
  • shapely input is returned unchanged.
  • EE-like input is converted via shapely.geometry.shape(obj.getInfo()).
  • Anything else raises a TypeError naming the expected type along with the exact conversion snippet.
• fit_geometry now accepts Union[shapely.geometry.base.BaseGeometry, ee.Geometry] and coerces before fitting.
• Unit tests for the EE-like paths.
• Docs coverage in quickstart.md, guide.md and faq.md.

Design decisions

• Duck typing, not isinstance(ee.Geometry) - Conversion keys off the presence of a callable getInfo, so we avoid a hard EE dependency in the conversion path.
• getInfo() runs outside the try - Only the shapely call is guarded; genuine EE runtime errors propagate instead of being masked as conversion failures.
• Narrow exception set, so that real geometry bugs aren't swallowed.
• Behavior for non-EE inputs is unchanged.

Testing

Added local unit tests: shapely passthrough, valid EE-like mock, invalid getInfo payload and unrelated input type.

NOTE: docs-check has pre-existing nitpicky failures on main. I believe this branch introduces no new warnings.

[jdbcode]

Thanks, @abidanBrito! Give me a bit of time to do a review. Note that it's expected that the build tests fail when run from a fork. I can run them once we get through any initial changes that are needed.

[abidanBrito]

Thanks, @abidanBrito! Give me a bit of time to do a review. Note that it's expected that the build tests fail when run from a fork. I can run them once we get through any initial changes that are needed.

Got it, thanks! I'll keep an eye out for any requested changes.

[jdbcode]

Looks greats, thanks so much!

[jdbcode]

@abidanBrito, while pulling into Google the lint checker found some minor style issues. I'll commit the fixes to this PR.

[abidanBrito]

Thanks for catching those, @jdbcode.

One thing I noticed - Running pixi run -e tests pyink --check --diff . flags more files than the ones I modified. Is that expected or am I holding it wrong?

Also, may I suggest adding pyink as a dev dependency and pre-commit hooks?

[jdbcode]

@abidanBrito, While reviewing and testing this internally, we had a discussion about the API design and wanted to run a proposal by you.

Currently, the implementation uses duck typing (checking for getInfo) to detect Earth Engine objects. While flexible and easy to test, this also accidentally (or intentionally, with missing allowed type) accepts ee.Feature and possibly ee.ComputedObjects.

We would like to propose restricting the supported types strictly to:

• shapely geometries (BaseGeometry)
• ee.Geometry

Why restrict it?

• Explicit over Implicit: An ee.Feature contains properties and metadata, and can even have a null geometry. If a user wants to fit a grid to a feature's geometry, we feel it's better for them to explicitly pass feature.geometry() rather than having Xee implicitly try to convert the feature.
• Type Safety: Restricting to ee.Geometry via isinstance checks prevents unexpected runtime errors if other ee.ComputedObject types are passed.

Proposed Implementation:

We can update _coerce_to_shapely_geometry to use explicit isinstance checks, e.g.,:

def _coerce_to_shapely_geometry(geometry):
  if isinstance(geometry, shapely.geometry.base.BaseGeometry):
    return geometry
  if isinstance(geometry, ee.Geometry):
    try:
      return shapely.geometry.shape(geometry.getInfo())
    except Exception as e:
      raise TypeError("Could not convert ee.Geometry...") from e
  raise TypeError("Expected shapely or ee.Geometry...")

(We would also update the tests to mock or use offline ee.Geometry objects).

How would you prefer to proceed?

1. If you agree with this direction, you are welcome to update the PR with these changes.
2. Alternatively, if you prefer, we can apply these updates (along with some minor code formatting adjustments required by our internal Google linters) directly to your branch.
3. Keep the existing implementation but be more inclusive/explicit with allowed types (discouraged for reasons above, but open to discussion).
4. Something else (open to discussion).

Let us know your thoughts.

[jdbcode]

Thanks for catching those, @jdbcode.

One thing I noticed - Running pixi run -e tests pyink --check --diff . flags more files than the ones I modified. Is that expected or am I holding it wrong?

Also, may I suggest adding pyink as a dev dependency and pre-commit hooks?

More Files Flagged: Yes, this is expected. There are some pre-existing files in the repo that don't fully conform to the style rules (or were formatted with a different version). You don't need to fix the entire repo—please only focus on the files you modified. You can target specific files to avoid the noise, for example:
pixi run -e tests pyink --check --diff xee/helpers.py xee/ext_test.py

Dev Dependencies & Pre-commit Hooks: pyink is already included as a test dependency in pyproject.toml. Adding pre-commit hooks to automate formatting is a great idea that will definitely help reduce commit churn and unexpected CI failures.

Before we move forward with a PR to add a .pre-commit-config.yaml, let's start with a feature request. Aligning external tools like pyink with Google's internal lint checks is tricky, and I need to do a little research first to see how we can better sync internal and external expectations.

Xee & External Contributions: Because Xee is mostly in maintenance mode for us, we're happy for and appreciate external PRs! The process for importing external code into Google's internal systems is currently a bit of a challenge, but we want to get the workflow streamlined. Thank you for bearing with us.

[abidanBrito]

Thanks a lot for the feedback, @jdbcode!

I completely agree with your points. Frankly, I tried to get smart with duck typing as a straight-forward way to avoid a hard dependency on ee during testing, but it came at the cost of type-safety. I didn't anticipate those edge cases, so I'm glad you caught this before it landed.

Also, I understand the need to align with your internal tooling and standards. I've pushed the changes. Hopefully it passes this time, but feel free to make any changes you deem necessary.