Skip to content

Fix/in point box for latlon #1207

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

debsankha
Copy link
Contributor

WHAT
This fixes #1203, a bug in Base.in(p::Point{🌐}, b::Box{🌐}).

HOW
Defining a new method for geometries on 🌐.

Copy link

codecov bot commented Jun 10, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.44%. Comparing base (ec0c5fe) to head (4914f6b).
Report is 5 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1207      +/-   ##
==========================================
+ Coverage   88.43%   88.44%   +0.01%     
==========================================
  Files         196      196              
  Lines        6157     6173      +16     
==========================================
+ Hits         5445     5460      +15     
- Misses        712      713       +1     

☔ 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
Member

@juliohm juliohm left a comment

Choose a reason for hiding this comment

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

First round of review attached.

Co-authored-by: Júlio Hoffimann <[email protected]>
@debsankha debsankha requested a review from juliohm June 11, 2025 09:21
@juliohm
Copy link
Member

juliohm commented Jun 11, 2025

Thank you @debsankha. Could you please elaborate on the inlonrange method? Appreciate it if you could call viz on every box you added to the tests to make sure they match what you have in mind.

@debsankha
Copy link
Contributor Author

Thank you @debsankha. Could you please elaborate on the inlonrange method?

Happy to do so. So the function is

function inlonrange(lonₗ, lonₚ, lonᵣ)
  if isnegative(lonₗ) && isnonnegative(lonᵣ)
     lonₚ  lonₗ || (isnonnegative(lonₚ) && lonₚ  lonᵣ)
  else
    lonₗ  lonₚ  lonᵣ
  end
end

The cases are:

  • If 0 <= lonₗ <= lonᵣ, the function returns lonₗ ≤ lonₚ ≤ lonᵣ, because the computation is the same as on the real line.
  • If 0 >= lonᵣ >= lonₗ, the same applies as above.
  • These two cases are handled by the else branch.
  • If 0 >= lonₗ and 0 <= lonᵣ, i.e. lonₗ and lonᵣ are on opposite sides of longitude 0 line, there are two cases
    • If lonₚ is on the negative side, inlonrange should be true if and only if it is within the interval [lonₗ, -180], which is equivalent to lonₚ ≤ lonₗ.
    • If lonₚ is on the positive side_, inlonrange should be true if and only if lonₚ ≤ lonᵣ.
    • These two cases are handled by the if branch.

Copy link
Member

@juliohm juliohm left a comment

Choose a reason for hiding this comment

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

Thank you @debsankha for the clear explanation and contribution ❤️

@juliohm juliohm merged commit e5e76ec into JuliaGeometry:master Jun 11, 2025
16 checks passed
@juliohm
Copy link
Member

juliohm commented Jun 11, 2025

Released a patch with the fix: JuliaRegistries/General#132717

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.

point in box with LatLon coordinates
2 participants