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

Rename bounding ‘box’ to ‘rect’; move structure to geo-types. #295

Merged
merged 1 commit into from
Jul 4, 2018

Conversation

frewsxcv
Copy link
Member

@frewsxcv frewsxcv commented Jul 4, 2018

  • Rename Bbox structure to Rect, move to geo_types, transition
    from four T values to two Coordinate<T> values
  • Rename BoundingBox to BoundingRect

Fixes #292.

@frewsxcv
Copy link
Member Author

frewsxcv commented Jul 4, 2018

bors r+

bors bot added a commit that referenced this pull request Jul 4, 2018
295: Rename bounding ‘box’ to ‘rect’; move structure to geo-types. r=frewsxcv a=frewsxcv

- Rename `Bbox` structure to `Rect`, move to `geo_types`, transition
  from four `T` values to two `Coordinate<T>` values
- Rename `BoundingBox` to `BoundingRect`

Fixes #292.

Co-authored-by: Corey Farwell <[email protected]>
Copy link
Member

@michaelkirk michaelkirk left a comment

Choose a reason for hiding this comment

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

Seeing some test failures:

   Compiling geo-types v0.1.1 (file:///Users/mkirk/src/georust/rust-geo/geo-types)
error[E0432]: unresolved import `algorithms::BoundingBox`
 --> geo-types/src/line_string.rs:5:18
  |
5 | use algorithms::{BoundingBox, EuclideanDistance};
  |                  ^^^^^^^^^^^ no `BoundingBox` in `algorithms`

error[E0407]: method `bbox` is not a member of trait `BoundingRect`
   --> geo-types/src/algorithms.rs:198:5
    |
198 | /     fn bbox(&self) -> Self::Output {
199 | |         get_bbox(self.0.iter().cloned())
200 | |     }
    | |_____^ not a member of trait `BoundingRect`

error[E0412]: cannot find type `Bbox` in this scope
   --> geo-types/src/algorithms.rs:147:44
    |
147 | fn get_bbox<I, T>(collection: I) -> Option<Bbox<T>>
    |                                            ^^^^ did you mean `Box`?

error[E0422]: cannot find struct, variant or union type `Bbox` in this scope
   --> geo-types/src/algorithms.rs:161:21
    |
161 |         return Some(Bbox {
    |                     ^^^^ did you mean `Box`?

error[E0432]: unresolved import `algorithms::BoundingBox`
 --> geo-types/src/line_string.rs:5:18
  |
5 | use algorithms::{BoundingBox, EuclideanDistance};
  |                  ^^^^^^^^^^^ no `BoundingBox` in `algorithms`

warning: unused import: `BoundingBox`
 --> geo-types/src/line_string.rs:5:18
  |
5 | use algorithms::{BoundingBox, EuclideanDistance};
  |                  ^^^^^^^^^^^
  |
  = note: #[warn(unused_imports)] on by default

error[E0407]: method `bbox` is not a member of trait `BoundingRect`
   --> geo-types/src/algorithms.rs:198:5
    |
198 | /     fn bbox(&self) -> Self::Output {
199 | |         get_bbox(self.0.iter().cloned())
200 | |     }
    | |_____^ not a member of trait `BoundingRect`

error[E0412]: cannot find type `Bbox` in this scope
   --> geo-types/src/algorithms.rs:147:44
    |
147 | fn get_bbox<I, T>(collection: I) -> Option<Bbox<T>>
    |                                            ^^^^ did you mean `Box`?

error[E0422]: cannot find struct, variant or union type `Bbox` in this scope
   --> geo-types/src/algorithms.rs:161:21
    |
161 |         return Some(Bbox {
    |                     ^^^^ did you mean `Box`?

warning: unused import: `BoundingBox`
 --> geo-types/src/line_string.rs:5:18
  |
5 | use algorithms::{BoundingBox, EuclideanDistance};
  |                  ^^^^^^^^^^^
  |
  = note: #[warn(unused_imports)] on by default

error[E0046]: not all trait items implemented, missing: `bounding_rect`
   --> geo-types/src/algorithms.rs:189:1
    |
131 |       fn bounding_rect(&self) -> Self::Output;
    |       ---------------------------------------- `bounding_rect` from trait
...
189 | / impl<T> BoundingRect<T> for LineString<T>
190 | | where
191 | |     T: CoordinateType,
192 | | {
...   |
200 | |     }
201 | | }
    | |_^ missing `bounding_rect` in implementation

error: aborting due to 5 previous errors

Some errors occurred: E0046, E0407, E0412, E0422, E0432.
For more information about an error, try `rustc --explain E0046`.
error: Could not compile `geo-types`.
warning: build failed, waiting for other jobs to finish...
error[E0046]: not all trait items implemented, missing: `bounding_rect`
   --> geo-types/src/algorithms.rs:189:1
    |
131 |       fn bounding_rect(&self) -> Self::Output;
    |       ---------------------------------------- `bounding_rect` from trait
...
189 | / impl<T> BoundingRect<T> for LineString<T>
190 | | where
191 | |     T: CoordinateType,
192 | | {
...   |
200 | |     }
201 | | }
    | |_^ missing `bounding_rect` in implementation

error: aborting due to 5 previous errors

Some errors occurred: E0046, E0407, E0412, E0422, E0432.
For more information about an error, try `rustc --explain E0046`.
error: Could not compile `geo-types`.

To learn more, run the command again with --verbose.

}
}

impl<T> BoundingRect<T> for Line<T>
Copy link
Member

Choose a reason for hiding this comment

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

Are the Line and LineString implementations intentionally duplicated in geo-types/src/algorithms.rs?

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, intentional. to have spade trait implementations for the types in geo-types, we need to have some basic algorithms included in the geo-types crate. but i'm planning to consolidate them in a follow-up pull request to this one.

@@ -174,7 +174,7 @@ where
type Point = Point<T>;

fn mbr(&self) -> ::spade::BoundingRect<Self::Point> {
let bbox = self.bbox();
let bbox = self.bounding_rect();
Copy link
Member

Choose a reason for hiding this comment

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

nit: seems like you're preferring let bounding_rect = elsewhere

Copy link
Member Author

Choose a reason for hiding this comment

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

yep, good catch, thanks

@frewsxcv
Copy link
Member Author

frewsxcv commented Jul 4, 2018

bors r-

@bors
Copy link
Contributor

bors bot commented Jul 4, 2018

Canceled

- Rename `Bbox` structure to `Rect`, move to `geo_types`, transition
  from four `T` values to two `Coordinate<T>` values
- Rename `BoundingBox` to `BoundingRect`

Fixes #292.
@frewsxcv frewsxcv force-pushed the frewsxcv-bounding-rect branch from 5a014c6 to a444fea Compare July 4, 2018 16:32
@frewsxcv
Copy link
Member Author

frewsxcv commented Jul 4, 2018

bors r+

bors bot added a commit that referenced this pull request Jul 4, 2018
295: Rename bounding ‘box’ to ‘rect’; move structure to geo-types. r=frewsxcv a=frewsxcv

- Rename `Bbox` structure to `Rect`, move to `geo_types`, transition
  from four `T` values to two `Coordinate<T>` values
- Rename `BoundingBox` to `BoundingRect`

Fixes #292.

Co-authored-by: Corey Farwell <[email protected]>
@bors
Copy link
Contributor

bors bot commented Jul 4, 2018

Build succeeded

@bors bors bot merged commit a444fea into master Jul 4, 2018
@frewsxcv frewsxcv deleted the frewsxcv-bounding-rect branch July 4, 2018 18:32
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