search

wkearn / RasterIO.jl

Simple Raster Formats for Julia
Other
18 stars 5 forks source link

Suggestions #6

Closed marciolegal closed 9 years ago

marciolegal commented 9 years ago

Hi, as posted here I have been playing around with the RasterIO package and am willing to contribute. I will start by writing some essential functions, such as ones dealing with projections, which I expect to attract more users quickly. In fact, I'm surprised theres such I small interest for Julia in the geospatial science community by now! Hope the rise of Geo/ organization can change this soom.

I have also noticed there's not much activity with this package for a while. Are you guys still on it?

Some initial suggestions fo the package as a whole

Actually I have done most of these things in my copy. Want to know your opinion on if what you think is viable for the master. The reference thing has one drawback: the user would need to destroy the raster and projection using fdestruction functions, unless theres a way to make the finalizer approach to work, which so far I couldn't. But that could be thought better later on.

I'll try to contribute by implementing the following funcionality in the near future (though cant tell when)

Marcio.

yeesian commented 9 years ago

Hey marciolegal, I notice you have forked this package, but it's not showing under your account, so I can't see the modifications you've made. Do you prefer to keep it private?

yeesian commented 9 years ago

I have also noticed there's not much activity with this package for a while. Are you guys still on it?

I had been focusing mostly on working with vector geometries so far, and (from what I recall) @wkearn hasn't had the time and the need to further develop this package.

We will be happy to have you help out -- maybe you might want to break down your suggestions into smaller issues/pull-requests? We use PRs to easily compare and discuss changes that have been made, e.g. https://github.com/wkearn/RasterIO.jl/pull/4/files.

The reference thing has one drawback: the user would need to destroy the raster and projection using fdestruction functions, unless theres a way to make the finalizer approach to work, which so far I couldn't.

Your suggestion to provide a handle to the raster dataset sounds like a promising approach -- perhaps we can start with that? Here's some resources for getting started:

  1. It is a long read, but you should go through the portion in the manual on calling C (and Fortran) libraries, while taking notes. You will need it over and over
  2. Here's an example of a type that provides a method similar to fdestruction, and registers it with the finalizer:
yeesian commented 9 years ago

I'll try to contribute by implementing the following funcionality in the near future (though cant tell when)

We can't promise to merge everything though, so it'll be easier for you to break it up into smaller changes, rather than risk the whole thing getting rejected

meggart commented 9 years ago

I am mostly working with raster data and would certainly also be interested in these changes. As yeesian said, it would be best to break down the changes into smaller changes.

It would certainly help if you could make your changes visible somehow, please let me know case you need any help with git/Github or creating pull requests etc.

wkearn commented 9 years ago

Hey guys,

I'm super swamped right now, and I'm not going to be able to contribute a whole lot to this package. @yeesian and @meggart should have push access to this repo now, and I'll try to review what I can. So far I think the progress you've made, @marciolegal, looks on track with what I had envisioned. The destruction of the GDAL object was a hurdle I never quite got over to loading rasters by reference, so if you can get that to work, it'll be a great step forward. Same thing with the projections, which I think is the first step to being able to use raster data in combination with other raster data and geospatial data more generally.

Glad to see things are moving forward. Wish I could help out more.

marciolegal commented 9 years ago

@yeesian, @meggart, @wkearn

Hi All,

Thank you for your responses and good to know you are still up to this. Regarding the changes, yes the intention is to propose changes/additions gradually, so we can refine them before they go to final. But It is good to know that we share similar ideas. The reason why you can't see my changes is because I lack experience with using Github: Julia Documentation suggests cheking out a branch and make a copy, while github suggests creating a fork. From your comments now I take that the best way to go is to create a fork, but can/should I prevent the public from having access to the unfinished/umpolished code? In addition, after creating a fork, what would be the best way to start working on it? Should I just clone it from within Julia as any regular unregistered Package?

yeesian commented 9 years ago

May I suggest taking some time to learn how to use git and github, e.g. using https://www.atlassian.com/git/tutorials?

Try experimenting with different things (it's okay, you won't be able to change any of the code here), and see how they work out, get comfortable with the commands, and it'll make more sense.

marciolegal commented 9 years ago

Regarding Changes:

I agree with @yeesian 's idea of starting by adding dataset handles to the Raster type. What do you think about the following format:

type Raster{T,N}
gdalhandle::GDALDatasetH
width::Int
height::Int
transform::Array{Float64,1}
projection::Projection
function Raster(gdalhandle,width,height,transform,projection,data)
   @assert size(data)[1:2] == (height,width)
   raster = new(gdalhandle,width,height,transform,projection,data)
end
end

With the following for Projection

type Projection
    string::ASCIIString
    spatialreference_handle::OGRSpatialReferenceH
  function Projection(string, spatialreference_handle)
    proj = new(string,spatialreference_handle)
  end
end

I havent made a constructor for Projection yet because it would involve specifying custom datums, etc. Gdal has a function to convert EPSG codes to WKT strings that could be used as a start. I dont think we should worry too much about a full constructor to Projections at the moment, as ppl rarely make their projection by hand.

The format above needs two things to work: a) an easy way to get data from disk. I was thinking about a way to make it work like myraster[ I1:I2 , J1:,J2, K1:K2, etc...]. So far I only have crude functions to get the data i needed. An uglier option is getdata(raster, parameters). b) A way to destroy the Raster object correctly when it is replaced by something else. As mentioned by @yeesian, the problem with those constructors is that OGRSpatialReferenceH and GDALDatasetH objects need to be finished with GDAL's destroy functions, which would imply implementing destroy(r::Raster) and destroy(p::Projection) and require the user to finish the objects, which is not very nice. A possible workaround would be to use finalizer to register the destroy functions to finalize the objects. I will read the docs provided by @yeesian and see if I can make it work.

yeesian commented 9 years ago

Hi @marciolegal, I can look into it over the weekend, and might be able to help with the finalizers/etc. But I haven't worked with GDAL/rasters before, and will appreciate some help.

Let's split up the development into 2 phases:

Phase I: Provide "low-level" functions to the corresponding C functions, in a file we call src/gdal_c.jl. These functions have essentially the same API as the GDAL API, and does nothing but passes its arguments to the corresponding ccall. Unfortunately, https://github.com/wkearn/RasterIO.jl/blob/master/src/GDALfuns.jl is too terse for me, since I (and other developers) will need the argument names to provide the context for how the function should be called. See work-in-progress at https://github.com/wkearn/RasterIO.jl/pull/7.

Phase II: When we define our own types, and the attributes they should have/etc.

Phase I

Will it be okay with you, on top of going through the reading list above, to help with

1. providing a few (proper) working examples, similar to the ones in http://www.gdal.org/gdal_tutorial.html, using only the functions from https://github.com/wkearn/RasterIO.jl/blob/master/src/GDALfuns.jl? To open a file, get the handle to some data, maybe do things with it, and then properly destroy/close it, the way you might in C/C++.

  1. copy the functions in https://github.com/wkearn/RasterIO.jl/blob/master/src/GDALfuns.jl to a new file, and name it gdal_functions.jl, and convert them into julia function calls, with docstrings/comments similar to those in http://www.gdal.org/gdal_8h.html. You don't have to do it for all of them; comment out all, and uncomment for the subset that you want to use, and have made progress on.

This is tedious work, but will help alot with subsequent development.

Remark 1: See the LibGEOS package as a running example: https://github.com/JuliaGeo/LibGEOS.jl/blob/master/src/geos_c.jl and https://github.com/JuliaGeo/LibGEOS.jl/blob/master/src/geos_functions.jl.

Remark 2 (optional/advanced): If you're into metaprogramming, macroexpand is your friend. Example:

julia> macro c(ret_type, func, arg_types, lib)
           local args_in = Any[ symbol(string('a',x)) for x in 1:length(arg_types.args) ]
           quote
               $(esc(func))($(args_in...)) = ccall( ($(string(func)), $(Expr(:quote, lib)) ), $ret_type, $arg_types, $(args_in...) )
           end
       end

julia> @c Cint GDALGetDataTypeSize (GDALDataType,) libgdal
GDALGetDataTypeSize (generic function with 1 method)

julia> macroexpand(:(@c Cint GDALGetDataTypeSize (GDALDataType,) libgdal))
quote  # none, line 4:
    GDALGetDataTypeSize(#8#a1) = begin  # none, line 4:
            ccall(("GDALGetDataTypeSize",:libgdal),Cint,(GDALDataType,),#8#a1)
        end
end

(Edited to reflect work-in-progress. See https://github.com/wkearn/RasterIO.jl/pull/7.)

meggart commented 9 years ago

Hi @marciolegal

From your comments now I take that the best way to go is to create a fork, but can/should I prevent the public from having access to the unfinished/umpolished code?

If you push some of your changes to your fork, they will be visible to the public, and the only way to prevent this would be to pay Github for private repositories. It is always possible to create a pull request and prefix the title with WIP which means that you are still working on the code but you invite the public to already comment on your ideas.

In addition, after creating a fork, what would be the best way to start working on it? Should I just clone it from within Julia as any regular unregistered Package?

Yes this is the way to go. Clone the package, then create a new branch (don't forget to checkout the branch) and do your edits.

meggart commented 9 years ago

Hi @yeesian and @marciolegal

as you might have noticed, the new version of Clang generates a wrapper without macros. I have re-run the wrapper and you can use these files if you want to get clearer function signatures:

https://github.com/meggart/GDALfuns.jl/blob/master/src/GDAL.jl https://github.com/meggart/GDALfuns.jl/blob/master/src/gdal_common.jl

yeesian commented 9 years ago

Thanks Fabian!

yeesian commented 9 years ago

closed by #8

@marciolegal, the thread here is getting swamped, and I suggest filing new issues for any further requests