Skip to content
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

Conversion from tileKey into extendedSpatialID #27

Closed
wants to merge 8 commits into from
Closed

Conversation

mi-24v
Copy link
Member

@mi-24v mi-24v commented Aug 1, 2024

feature

  • tileKey->extendedSpatialId conversion via ConvertQuadkeysAndAltitudekeysToExtendedSpatialIDs
  • documentation for ConvertQuadkeysAndAltitudekeysToExtendedSpatialIDs

@mi-24v mi-24v added documentation Improvements or additions to documentation enhancement New feature or request labels Aug 1, 2024
@mi-24v mi-24v self-assigned this Aug 1, 2024
Copy link
Member

@HarutakaMatsumoto HarutakaMatsumoto left a comment

Choose a reason for hiding this comment

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

I reviewed it.
Check it out.

extendedSpatialID.SetY(int64(mapTile.Y))
extendedSpatialID.SetZ(z)
extendedSpatialID.SetZoom(r.QuadkeyZoom(), r.AltitudekeyZoom())
extendedSpatialIDs = append(extendedSpatialIDs, *extendedSpatialID)
Copy link
Member

Choose a reason for hiding this comment

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

ConvertExtendedSpatialIDsToQuadkeysAndVerticalIDs(_) returns a unique slice.
For the consistency, make extendedSpatialIDs unique.

// }
//
// extendedSpatialIDs :["20/85263/65423/23/-2", "20/85263/65423/23/-1"]
func ConvertQuadkeysAndAltitudekeysToExtendedSpatialIDs(request []*object.FromExtendedSpatialIDToQuadkeyAndAltitudekey) ([]object.ExtendedSpatialID, error) {
Copy link
Member

Choose a reason for hiding this comment

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

This is a parallel function of ConvertQuadkeysAndVerticalIDsToExtendedSpatialIDs(_).
So the parameters should be similar.
However, outputHZoom and outputVZoom are missing.
Also, QuadkeyAndAltitudekey should be used instead of FromExtendedSpatialIDToQuadkeyAndAltitudekey.
Execute extendedSpatialIDCheckZoom(_) and quadkeyCheckZoom(_).
Please fix this.

return extendedSpatialIDs, nil
}

func convertAltitudeKeyToZ(altitudekey int64, altitudekeyZoomLevel int64, zZoomLevel int64, zBaseExponent int64, zBaseOffset int64) (int64, int64, error) {
Copy link
Member

Choose a reason for hiding this comment

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

We decided to make these converters public.
Make convertAltitudeKeyToZ(_) and convertZToAltitudeKey(_) public.

Copy link
Member

Choose a reason for hiding this comment

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

Why do you change all of this?

Copy link
Member Author

Choose a reason for hiding this comment

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

It seems to be CRLF -> LF conversion was made in e5825d39
using ignore whitespace will suppress too many diff
need fix?

@mi-24v
Copy link
Member Author

mi-24v commented Aug 16, 2024

it founds that QuadkeysAndAltitudekeys should not be used in new feature (#28 )
This PR will be closed and replacement Pull Request is to considered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants