Skip to content

PR - fix several outstanding PR's without CLA. #132

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

PR - fix several outstanding PR's without CLA. #132

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
97dcaea
ppc64le - Fix deviation in floating point values caused by FMA
rsned Mar 28, 2025
1cfb753
Fix godoc url
rsned Mar 28, 2025
feba0ab
Fix a few random outstanding minor lint issues.
rsned Mar 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ to geography are not part of the core library (e.g. easting/northing
conversions, ellipsoid approximations, geodetic vs. geocentric coordinates,
etc).

See http://godoc.org/github.com/golang/geo for specific package documentation.
See http://pkg.go.dev/github.com/golang/geo for specific package documentation.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Most of this does not match the PR description.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There are 3 commits that have gotten smashed together. I'm still trying to figure out the mapping from how easy things worked in g4 to how less intuitive git works. ( I'm still trying to undo a commit == a CL model of things)

This was an attempt to patch in several pull requests that did NOT have CLA so they could be all put in and get closed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This was an attempt to patch in several pull requests that did NOT have CLA so they could be all put in and get closed.

I don't think we can do that.

https://cla.developers.google.com/about/google-individual#:~:text=Should%20You%20wish,party%3A%20%5Bnamed%20here%5D%22.

#91 should be good to merge, if it's really the right solution.

I will try to get the others into shape.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm still trying to figure out the mapping from how easy things worked in g4 to how less intuitive git works. ( I'm still trying to undo a commit == a CL model of things)

You can probably fine some p4/git cheat sheets to help. You might want to try jj, since it's more Mercurial-like.


For an analogous library in C++, see https://github.com/google/s2geometry, in
Java, see https://github.com/google/s2-geometry-library-java, and Python, see
Expand All @@ -98,36 +98,36 @@ Legend:
* 🟡 - Mostly Complete
* ❌ - Not available

## [ℝ¹](https://godoc.org/github.com/golang/geo/r1) - One-dimensional Cartesian coordinates
## [ℝ¹](https://pkg.go.dev/github.com/golang/geo/r1) - One-dimensional Cartesian coordinates

C++ Type | Go
:--------- | ---
R1Interval | ✅

## [ℝ²](https://godoc.org/github.com/golang/geo/r2) - Two-dimensional Cartesian coordinates
## [ℝ²](https://pkg.go.dev/github.com/golang/geo/r2) - Two-dimensional Cartesian coordinates

C++ Type | Go
:------- | ---
R2Point | ✅
R2Rect | ✅

## [ℝ³](https://godoc.org/github.com/golang/geo/r3) - Three-dimensional Cartesian coordinates
## [ℝ³](https://pkg.go.dev/github.com/golang/geo/r3) - Three-dimensional Cartesian coordinates

C++ Type | Go
:------------ | ---
R3Vector | ✅
R3ExactVector | ✅
Matrix3x3 | ✅

## [S¹](https://godoc.org/github.com/golang/geo/s1) - Circular Geometry
## [S¹](https://pkg.go.dev/github.com/golang/geo/s1) - Circular Geometry

C++ Type | Go
:----------- | ---
S1Angle | ✅
S1ChordAngle | ✅
S1Interval | ✅

## [S²](https://godoc.org/github.com/golang/geo/s2) - Spherical Geometry
## [S²](https://pkg.go.dev/github.com/golang/geo/s2) - Spherical Geometry

### Basic Types

Expand Down
2 changes: 1 addition & 1 deletion s2/cell_index_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ func verifyCellIndexContents(t *testing.T, desc string, index *CellIndex) {
// Verify that the cell does not intersect the range.
if x.cellID.RangeMin() <= r.LimitID().Prev() &&
x.cellID.RangeMax() >= r.StartID() {
t.Errorf("%s: CellID does not interect the current range: %v <= %v && %v >= %v", desc, x.cellID.RangeMin(), r.LimitID().Prev(), x.cellID.RangeMax(), r.StartID())
t.Errorf("%s: CellID does not intersect the current range: %v <= %v && %v >= %v", desc, x.cellID.RangeMin(), r.LimitID().Prev(), x.cellID.RangeMax(), r.StartID())
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions s2/edge_distances_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,26 +202,26 @@ func TestEdgeDistancesUpdateMinInteriorDistanceRejectionTestIsConservative(t *te
a: Point{r3.Vector{X: 1, Y: -8.9031850507928352e-11, Z: 0}},
b: Point{r3.Vector{X: -0.99999999999996347, Y: 2.7030110029169596e-07, Z: 1.555092348806121e-99}},
minDist: minDist,
want: false,
want: true,
},
{
x: Point{r3.Vector{X: 1, Y: -4.7617930898495072e-13, Z: 0}},
a: Point{r3.Vector{X: -1, Y: -1.6065916409055676e-10, Z: 0}},
b: Point{r3.Vector{X: 1, Y: 0, Z: 9.9964883247706732e-35}},
minDist: minDist,
want: false,
want: true,
},
{
x: Point{r3.Vector{X: 1, Y: 0, Z: 0}},
a: Point{r3.Vector{X: 1, Y: -8.4965026896454536e-11, Z: 0}},
b: Point{r3.Vector{X: -0.99999999999966138, Y: 8.2297529603339328e-07, Z: 9.6070344113320997e-21}},
minDist: minDist,
want: false,
want: true,
},
}

for _, test := range tests {
if _, ok := UpdateMinDistance(test.x, test.a, test.b, test.minDist); !ok {
if _, ok := UpdateMinDistance(test.x, test.a, test.b, test.minDist); ok != test.want {
t.Errorf("UpdateMinDistance(%v, %v, %v, %v) = %v, want %v", test.x, test.a, test.b, test.minDist, ok, test.want)
}
}
Expand Down
4 changes: 2 additions & 2 deletions s2/latlng.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,15 +64,15 @@ func (ll LatLng) Distance(ll2 LatLng) s1.Angle {
lng1, lng2 := ll.Lng.Radians(), ll2.Lng.Radians()
dlat := math.Sin(0.5 * (lat2 - lat1))
dlng := math.Sin(0.5 * (lng2 - lng1))
x := dlat*dlat + dlng*dlng*math.Cos(lat1)*math.Cos(lat2)
x := float64(dlat*dlat) + float64(dlng*dlng*math.Cos(lat1)*math.Cos(lat2))
return s1.Angle(2*math.Atan2(math.Sqrt(x), math.Sqrt(math.Max(0, 1-x)))) * s1.Radian
}

// NOTE(mikeperrow): The C++ implementation publicly exposes latitude/longitude
// functions. Let's see if that's really necessary before exposing the same functionality.

func latitude(p Point) s1.Angle {
return s1.Angle(math.Atan2(p.Z, math.Sqrt(p.X*p.X+p.Y*p.Y))) * s1.Radian
return s1.Angle(math.Atan2(p.Z, math.Sqrt(float64(p.X*p.X)+float64(p.Y*p.Y)))) * s1.Radian
}

func longitude(p Point) s1.Angle {
Expand Down
4 changes: 1 addition & 3 deletions s2/lexicon.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,7 @@ func (l *sequenceLexicon) add(ids []int32) int32 {
if id, ok := l.idSet[hashSet(ids)]; ok {
return id
}
for _, v := range ids {
l.values = append(l.values, v)
}
l.values = append(l.values, ids...)
l.begins = append(l.begins, uint32(len(l.values)))

id := int32(len(l.begins)) - 2
Expand Down
2 changes: 1 addition & 1 deletion s2/metric_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func TestMetric(t *testing.T) {
t.Errorf("MaxEdgeAspect = %v, want >= 1", MaxEdgeAspect)
}
if got := MaxEdgeMetric.Deriv / MinEdgeMetric.Deriv; MaxEdgeAspect > got {
t.Errorf("Edge Aspect: %v/%v = %v, want <= %v", MaxEdgeMetric.Deriv, MinEdgeMetric.Deriv, got, MaxDiagAspect)
t.Errorf("Edge Aspect: %v/%v = %v, want <= %v", MaxEdgeMetric.Deriv, MinEdgeMetric.Deriv, got, MaxEdgeAspect)
}
if MaxDiagAspect < 1 {
t.Errorf("MaxDiagAspect = %v, want >= 1", MaxDiagAspect)
Expand Down
2 changes: 1 addition & 1 deletion s2/query_options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ func (q *queryOptions) MaxError(x s1.ChordAngle) *queryOptions {
// This must be at least 1.
func (q *queryOptions) MaxResults(x int) *queryOptions {
// TODO(roberts): What should be done if the value is <= 0?
q.maxResults = int(x)
q.maxResults = x
return q
}

Expand Down