looser ImageCore and ColorTypes deps #207
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thank you! |
yeesian
added a commit
that referenced
this pull request
Jun 24, 2021
* ignore .DS_Store files * Return errors for unknown datatypes when looking up gdaltypes * bugfix for looking up julia datatypes when given a gdaldatatype * throw informative error message for unknown field types * Add instructions for managing dependencies * remove cenum dependency * [Breaking change] Maintain our own OGRFieldType This will be part of a broader move away from GDAL's enums so that the interface for ArchGDAL is "closed" (i.e. takes and return ArchGDAL types) and not require users to import GDAL. * We need to be careful to maintain interoperability with GDAL's enums, so I've defined corresponding convert() methods, and methods to map between the different types. * I also took the opportunity to update the _FIELDTYPE dictionary now that I see how it's used (in `src/tables.jl`), and will commit the code for supporting the corresponding types as part of the same PR. * [stylistic] Require explicit return statements and enforce maxcolumnwidth=80 1. When writing the return statements, return should be at the top-level, rather than being nested inside e.g. an if-statement. * The convention of explicit returns is something that's been on my mind for a long time, and I think it's net healthier (when developing code) to be explicit about what's being returned to reduce on the number of gotchas when using it (e.g. in #171). 2. Return `nothing` upon destroying a GDAL object (rather than allowing users to encounter and depend on potentially dangerous behavior per https://www.hyrumslaw.com/) 3. The max columnwidth of 80char is somewhat arbitrary but most of the codebase is already in alignment, so I just want to bring it to 100% to reduce on the number of stylistic rules that we're still ambiguous about. * Update the types in the API to be based on julia types (rather than C-types or GDAL-types) * [Breaking change] Introduce types for wkbGeometryType * Encapsulate the tests in their own testsets for better organization of test error reports * Refactor src/types.jl (i) move constants into its own file (ii) use a macro for generating conversion methods between enums * annotate function signatures with return types * [Breaking] copywholeraster -> copywholeraster! It now returns the dataset that was copied into as the return type. * annotate with return types * [Breaking] Deprecate serializeJSON It returns a Ptr{Void}, and feels sufficiently unsafe for people to work with. (I initially thought it might return a String.) We don't have any tests for it, and I haven't seen widespread use for it through GDAL, so it's safer to deprecate this function until I've thought through how to make it accessible to users. * [Breaking] _dataset_type -> pixeltype We also use it in the rasterio functions for safe behavior, rather than assuming that the bands all have the same pixeltype. * Annotate with return types * Fix the implementation of src/tables.jl We should be using the interface based on https://tables.juliadata.org/stable/#Tables.AbstractRow-1, rather than overriding base methods. Along the way, I realize that the methods for getfield(feature, name) and getgeom(feature, name) do not handle the case when the name is invalid, so I've added implementations where they are missing, and return nothing/C_NULL for existing implementations. I'll use coverage results later to see where to augment with tests. * Annotate with return types * tighten the return type * white spacing * use isnothing() * whitespacing * Array -> Vector * Introduce enums for WKBByteOrder * remove pointer definitions in constants.jl They were useful in a period of time when all GDAL objects are just Ptr{Void} and hence there was no way to distinguish between the type of GDAL object if we did not wrap the pointer in one of those constants. Ever since then, we have introduced corresponding mutable structs in types.jl that have obviated the need for using pointers directly. * [Breaking] Provide proper support for all GDAL enums This uses the enum macro in Base to define each of the GDAL enums. * I decided to maintain names with GDAL for the most part to maintain searchability for the same names. (This is a breaking change for some of the existing names, e.g. for GDALOpenFlag.) * I decided not to go with cenum because I didn't want to support having different enum values evaluate to the same UInt8. * We no longer expose internal dicts of mappings between data types. Now everything should go through base conversion. * I verified (by a search on `::GDAL.` that the only remaining GDAL.jl types in the API for this package are GDAL.GDALColorEntry, GDAL.OGREnvelope, and GDAL.OGREnvelope3D. As they are well-defined structs, I see no compelling reason to wrap them yet. * [Breaking] We now use Base conversion for mapping between the different data types, rather than having gdaltype(), datatype(), etc. * Cleaned up the implementation of rasterio!() to use GDAL.jl's functions rather than doings its own ccall. * improve test coverage for drivers * Remove unnecessary imports of GDAL in the tests * Add tests for geotransforms And fix a bug with the implementation for `composegeotransform!()`. * whitespacing * [Breaking] Update implementation of GeoInterface.coordinates() Coordinate dimensions should now only return 2 or 3. * LinearRings have the strange property of being treated like linestrings Their geotype returns LineString even though the WKT indicates otherwise, and I have seen reports such as https://trac.osgeo.org/gdal/ticket/1755, but don't think it'll get resolved in this package. For now I'm just going to report what GDAL returns. * Implement a basetype() method * Remove nextnamedtuple() from tables.jl It is not part of the Tables.jl interface. (We can provide a more general conversion from Features to NamedTuples as a feature request if desired.) * Add tests for tables.jl * Add tests for rasterio!(...) Spotted an indexing bug and fixed it along the way. * Add tests for raster/array.jl * [Breaking] Remove untested method for addfielddefn() * Add tests for displays of different objects * Add test for unsafe_getgeom(feature) * Improve test coverage for raster/array.jl * add tests for tables.jl * add prefix * Remove obsolete function It has been overridden by the following function further below: ``` write!( dataset::T, buffer::Array{<:Real, 3}, indices, rows::UnitRange{<:Integer} = 1:height(dataset), cols::UnitRange{<:Integer} = 1:width(dataset), )::T where {T <: AbstractDataset} ``` * Add support for displaying color tables * got missed in the previous commit * Delete obsolete function It has an alternative implementation below, which is exercised in the unit test. * Whitespacing * Add test for reproject in the case of a no-op * Add test for copywholeraster! * Add test for unsafe_getgeom(feature, name) * Add test for unsafe_copy(layer) * Add tests for cloning null geometries * Update the documentation for working with the tables interface * Update API reference for geotransfromations * Add README for building the documentation * Fix command for activating the environment * Update macro name to reflect its usage and provide a docstring * Add support for images Based on the current snapshot of https://github.com/yeesian/ArchGDAL.jl/tree/images. (I got drawn into this from wanting to have proper support for GDAL.GDALColorEntry.) * Add the return keyword * Add support for YCbCr * [Breaking] Delete unneeded ColorTable methods The single most useful thing from the colortable is the palette interpretation. Rather than encouraging operations on colortables (with very limited functionality from GDAL), users are better served by arrays of ColorTypes using Colors.jl, e.g. (http://juliagraphics.github.io/Colors.jl/stable/colormapsandcolorscales/#Generating-a-range-of-colors) ``` range(c1, stop=c2, length=15)` ``` instead of ``` createcolorramp!(colortable, startindex, startcolor, endindex, endcolor) ``` * Add tests for uncovered methods of features Namely, * getgeom(feature) * getgeom(feature, i) * unsafe_getgeom(feature) * unsafe_getgeom(feature, i) * getstyletable(feature) * Add support for options in toJSON * Increase test coverage for NULL geometries * Add support for kwargs in lineargeom * [Breaking] Remove initialize!(stylemanager, feature) Use initialize!(stylemanager, getstylestring(feature)) instead. * Use summarize(io, rasterband) for NULL bands * remove obsolete comment * Update README.md * Add tests for raster attribute tables and color tables * Add tests for ColorTable <-> RAT * Add test for cloning raster attribute tables * Fix flakey test Caught in CI for windows-latest - x64: ``` Create a Point: Test Failed at D:\a\ArchGDAL.jl\ArchGDAL.jl\test\test_geometry.jl:45 Expression: AG.toJSON(point, ["SIGNIFICANT_FIGURES=1"]) == "{ \"type\": \"Point\", \"coordinates\": [ 1e+02, 7e+01, 0.0 ] }" Evaluated: "{ \"type\": \"Point\", \"coordinates\": [ 1e+002, 7e+001, 0.0 ] }" == "{ \"type\": \"Point\", \"coordinates\": [ 1e+02, 7e+01, 0.0 ] }" ``` * Add tests for conversion errors * Add test to exercise `macro ogrerr(code, message)` * Add test to exercise `macro cplerr(code, message)` * Add tests for imread * Add tests for axisorder when importing CRS * Add tests for CRS conversion * whitespacing * [Breaking] Remove destroy(driver) We do not provide any ways of creating drivers in this package and should not be in the business of destroying them. * Replace tabs with spaces I didn't realize my text editor was using tabs earlier * Add tests for images Removed default support for HSL rasters since I haven't seen any examples in the wild of it actually working from GDAL, and we'll benefit from seeing actual files if someone actually wants to use it and runs into the lack of support for it. * Remove untested code for HSL * Test for error cases * Add test for gray colortable * Add test cases for NULL SpatialRefs * Add test for color relief * Add test for conversion error * Add test for displaying dataset with many feature layers * Use format from JuliaFormatter See the README of https://github.com/domluna/JuliaFormatter.jl * Add github workflow for running JuliaFormatter * Add format configurations * Add a format checker for PRs * Update the README with the style guide * Restrict the format checker to just the src/ directory for now. It seems to error on test/test_gdalutilies.jl even though it still formats the files. We'll still have a format_pr.yml to autoformat the entire repository, so I'm less concerned about the test directory. * Update README.md * tweak phrasing * Include Julialang in the statement of support * provide a README.md for the tests In particular, documenting how to work with data (if needed). * Fix the return type * fix format * Tweak README * Add functionality and documentation for raster images * Fix file formatting * Allow to use interactive Datasets in unsafe_gdal functions in utilities.jl * Replace Pkg.PlatformEngine --> Custom download & verify * Move importing of Downloads and SHA to remotefiles.jl * Add contributor's guide * Ran JuliaFormatter * Fix tables example * ArchGDAL.getgeomdefn -> getgeomdefn * add a section about the compat entries These are needed, so that Julia can resolve the package versions and it is good practice to have compat entries for all dependencies. Co-authored-by: Felix Cremer <felix.cremer@uni-jena.de> * add compat entries for ColorTypes and ImageCore * Add badges * rearrange badges for color * fix for table geometry v.s. field ordering * Add conversion for geometries to wkbUnknown This is useful when constructing vectors of geometries of multiple types * ran JuliaFormatter * Revamp implementation of imread() to be on-par with read() Needs corresponding unit tests * Add tests for images * Ran JuliaFormatter * remove obsolete code * Fix the description for updating compatibility * Removing the forecast checker from PRs It'll be easier to let PR authors not worry about the format for multi-commit PRs, and wait for text editor integration instead. In the meantime, there's a format_pr.yml that can autoformat the codebase * Re-enable test for gdal warp * Use JuliaFormatter * Use links instead of URLs * Use `=== nothing` instead of isnothing() for performance See JuliaLang/julia#36444 for context. * Future-proof the url to use the latest 1.* version. * Use the project environment for the docs * Format test cases that take up too much vertical whitespace * Bugfix to write out the file before reading it. * Remove unnecessary `Table{T<:AbstractFeatureLayer}` type. FeatureLayers now implement the table interface without having to go through a Table wrapper type. * Undo replacement of empty geometry column names with "geometry". If we just do the replacement in the schema, the schema will be out of sync with the geometry column names of the individual features in the layer: this causes issues that were not detected when it was first introduced in #160. * Tweak wording * format file * fix typo and use https where possible * use abstract input types and fix indentation for multiline strings * remove conventions.md * Return missing rather than -1 or nothing if no matching field is found. * Added discussion on tables interface Based on the thread in #179 (comment). * Add discussion of enum values between base enums and cenums Based on the discussion in #179 (comment). * Updating getnodatavalueband to return missing rather than nothing Based on https://docs.julialang.org/en/v1/manual/faq/#Nothingness-and-missing-values-1: > To represent missing data in the statistical sense (NA in R or NULL in SQL), use the missing object. * Capture the discussion on colors and images * Ran juliaformatter * Add docstrings for enum conversions * annotate return types * Update test-specific dependencies Based on https://pkgdocs.julialang.org/v1/creating-packages/#Test-specific-dependencies-in-Julia-1.2-and-above. * Fix doc builds * ignore tif files * Be consistent in using module names as prefixes * add GDAL as dependency for docs * Update instructions for formatting code. * Add details on AbstractDataset potentially having multiple tables * Add a note about (lack of) support for spatialite * Add an instruction for starting julia * Add note that tables are only for OGR features and geometries * rewrite if-else to have a single return * Update show methods to return nothing * run JuliaFormatter * Add support for GPI_RGB with GCI_PaletteIndex * looser ImageCore and ColorTypes deps (#207) * Switch from `missing` to `nothing` This is for two reasons: 1. The nodata value isn't meant to be used as a "sentinel value" in the raster output. If ArchGDAL wants to provide a convenience function for returning `Array{Union{T, Missing}}`, the treatment of missing data should be based on the appropriate use of `getmaskband()` (https://trac.osgeo.org/gdal/wiki/rfc15_nodatabitmask for details). 2. Even if ArchGDAL does provide a convenience function for returning `Array{Union{T, Missing}}`, the proper place for using `missing` values is in the array being constructed, and not the value being returned from `getnodatavalue(band)`. * Return `nothing` rather than `missing` when no matching field is found * formatting Co-authored-by: Felix Cremer <felix.cremer@uni-jena.de> Co-authored-by: Aayush Joglekar <aayushjog@gmail.com> Co-authored-by: Rafael Schouten <rafaelschouten@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There are still packages with older dependencies, and these should work equally well here?