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

Improve scaling code a bit #152

Merged
merged 1 commit into from
Aug 8, 2023
Merged

Improve scaling code a bit #152

merged 1 commit into from
Aug 8, 2023

Conversation

nyurik
Copy link
Member

@nyurik nyurik commented Aug 8, 2023

  • On a second though, I think to_mvt_unscaled is more accurate. raw here would be confusing.
  • There is no need to store bounds and do additional math ops on it. Can precompute a few things earlier on.
  • Default can be auto-derived

P.S. @pka I think its better to push PRs to individual's forks, and only keep project-maintained versions in the primary repo. This way there would be no experimental branches in the main repo, reducing confusion.

* On a second though, I think `to_mvt_unscaled` is more accurate. `raw` here would be confusing.
* There is no need to store bounds and do additional math ops on it. Can precompute a few things earlier on.
* Default can be auto-derived
@nyurik nyurik requested a review from pka August 8, 2023 18:36
@nyurik
Copy link
Member Author

nyurik commented Aug 8, 2023

P.S. Once you +1 this (or merge it into your branch), i can +1/merge yours into main

@nyurik nyurik merged commit a0f937a into georust:mvttrans Aug 8, 2023
1 check passed
@nyurik nyurik deleted the mvt-trans2 branch August 8, 2023 23:13
nyurik added a commit that referenced this pull request Aug 9, 2023
* On a second though, I think `to_mvt_unscaled` is more accurate. `raw`
here would be confusing.
* There is no need to store bounds and do additional math ops on it. Can
precompute a few things earlier on.
* Default can be auto-derived
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.

2 participants