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

[breaking] Less strict in types which we accept and unified indices/coords behaviour. #79

Merged
merged 3 commits into from
Oct 1, 2021

Conversation

evetion
Copy link
Owner

@evetion evetion commented Sep 5, 2021

Refactored indices and coords behaviour. Removed centercoords variants, replaced them with strategy argument in relevant methods.
Default behaviour is now to use Center everywhere.

Improved online documentation by docstrings and added README to it.

Fixes #77

Refactored `indices` and `coords` behaviour. Removed `centercoords` variants, replaced them with `strategy` argument in relevant methods.
Default behaviour is now to use `Center` everywhere.

Improved online documentation by docstrings and added README to it.
@evetion evetion mentioned this pull request Sep 5, 2021
Comment on lines 9 to 11
A::AbstractArray{T,3}
f::AffineMap
crs::WellKnownText{GeoFormatTypes.CRS,<:String}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm none of the three fields are concrete types, right? Is there a reason you went with that? Since normally it's best to avoid such that the compiler can specialize: https://docs.julialang.org/en/v1/manual/performance-tips/#Avoid-fields-with-abstract-type

Copy link
Owner Author

Choose a reason for hiding this comment

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

A is either an ArchGDAL IDataSet or an Array{T}.
AffineMap just isn't concrete, it's subs (linear and translation can be any kind of array, often StaticArrays, but not always).
WellKnownText also isn't concrete.

I'm happy to take suggestions how to resolve this.

@evetion evetion merged commit 0e89f95 into master Oct 1, 2021
@visr visr deleted the feature/less-types branch January 12, 2022 20:20
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.

BBox is Float or Int
2 participants