8.0 stable #388
8.0 stable #388
Conversation
Fix tests with to_s comparison
There was a problem hiding this comment.
Thanks for submitting this @Beardev118. Like I mentioned in my comment the goal is to be able to compare objects directly and not attributes since that used to work, but isn't anymore. A good first step would be identifying which attributes are different from each other and we can then work towards identifying the issue.
test/cases/basic_test.rb
Outdated
| assert_equal factory.point(0, 0).x, obj.default_latlon.x | ||
| assert_equal factory.point(0, 0).y, obj.default_latlon.y |
There was a problem hiding this comment.
I think the bigger issue we're trying to look at here is that we should be able to compare by rgeo objects and not their attributes.
assert_equal factory.point(0,0), obj.default_latlonshould work but it's not for some reason.
There was a problem hiding this comment.
Fixed this issue
Fixed point object compare issue
test/cases/basic_test.rb
Outdated
| area_p = MyPolygon.new(area) | ||
| obj_area_p = MyPolygon.new(object.area) | ||
| assert_equal area_p, obj_area_p |
There was a problem hiding this comment.
We also need to change this back to comparing area and object.area directly. We can remove the MyPolygon class as well.
Removed MyPolygon class and fixed object directly compare issue
|
Hello, Keith |
|
It's only failing on postgres 11. I assume it has something to do with not being able to cast the WKT for the default value to the proper SRID now that it's been changed to 3857. Thinking about this more I think the solution is to change the SRID of that column back to 0 and instead modify the def factory(srid: 3857)
RGeo::Cartesian.preferred_factory(srid: srid)
endAnd when we're comparing data to the assert_equal factory(srid: 0).point(0, 0), obj.default_latlon |
|
Should be a similar solution to the other test. Just make sure the factory we're generating features with is the same SRID of the column. Thanks! |
updated the factory for srid match
|
All workflow passed
|
|
Thanks @Beardev118 this looks good. Last question is any reason this is pointing to 8.0-stable rather than main? |
|
Thank you for your response
You wrote: "In the update to version 8, a few tests have to use to_s to
compare result geometries instead of the objects directly. We should be
able to compare the objects directly, though."
So, I started with 8.0-stable branch, but it can be applied to the main
branch
If you want, I'll edit it to the main branch
…On Mon, Dec 4, 2023 at 10:37 PM Keith Doggett ***@***.***> wrote:
Thanks @Beardev118 <https://github.com/Beardev118> this looks good. Last
question is any reason this is pointing to 8.0-stable rather than main?
—
Reply to this email directly, view it on GitHub
<#388 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ANIME564R2R6LBGLS4ZN2XDYHY7CDAVCNFSM6AAAAAA7IFIXHSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMZZGUZDANBQGM>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
Thank you! LGTM merging |
|
You're welcome!
…On Wed, Dec 6, 2023 at 12:19 AM Keith Doggett ***@***.***> wrote:
Thank you! LGTM merging
—
Reply to this email directly, view it on GitHub
<#388 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ANIME525L4CPPIQWHXA2J2LYH6TZ3AVCNFSM6AAAAAA7IFIXHSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNBRG44DIMRUGU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
What does this PR do?
to_scomparison #359