-
Notifications
You must be signed in to change notification settings - Fork 18
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
More show improvements #185
Conversation
this is probably the best example of a 9pm commit I've had so far... in all seriousness we could change the name
This allows the new show methods to also work in e.g arrays.
then, you can actually copy/paste the code
1d6591d
to
718f5a3
Compare
CI is finally green!! |
@@ -80,7 +80,7 @@ test_display(pointm, "Point{false, true}((1, 2, 3))", "Point((1,2,3))") | |||
pointm_crs = GI.Point((X=1, Y=2, M=3); crs=EPSG(4326)) | |||
@test parent(pointm_crs) === parent(pointm) | |||
@test GI.crs(pointm_crs) === EPSG(4326) | |||
test_display(pointm_crs, "Point{false, true}((1, 2, 3), crs = EPSG{1}((4326,)))", "Point((1,2,3))") | |||
test_display(pointm_crs, "Point{false, true}((1, 2, 3), crs = \"EPSG:4326\")", "Point((1,2,3))") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if showing EPSG as a string is confusing to users who still need to use the constructor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, but it's equivalent in most cases that one might care about (Proj and GDAL)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should change show on GFT types to show the constructor too?
Supersedes #177
Fix #176 #182
actual changes
bugfixes and tests