need update to core.create_location

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

need update to core.create_location

Michael Barton
In the GRASS python library, core.create_location needs to be updated to use the new datum argument in g.proj.

I think I know how to update it, but it looks like there may be an error in the create_location code. It is using "datum" to refer to the "datumtrans" argument of g.proj. 

Currently, the code looks like this

def create_location(dbase, location,
                    epsg = None, proj4 = None, filename = None, wkt = None,
                    datum = None, desc = None):

...
    
    kwargs = dict()
    if datum:
        kwargs['datum'] = datum

I think it probably should be:

def create_location(dbase, location,
                    epsg = None, proj4 = None, filename = None, wkt = None,
                    datum = None, datumtrans=None, desc = None):

...
    
    kwargs = dict()
    if datum:
        kwargs['datum'] = datum
    if datumtrans:
        kwargs['datumtrans'] = datumtrans

The programmers manual would need to be updated to reflect this change. This means a change in the arguments for create_location. But it is currently sending the wrong argument to g.proj anyway. So this is also a bug fix to create_location.



Michael
____________________
C. Michael Barton
Director, Center for Social Dynamics & Complexity 
Professor of Anthropology, School of Human Evolution & Social Change
Arizona State University

voice:  480-965-6262 (SHESC), 480-727-9746 (CSDC)
fax:          480-965-7671 (SHESC),  480-727-0709 (CSDC)












_______________________________________________
grass-dev mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/grass-dev
Reply | Threaded
Open this post in threaded view
|

Re: need update to core.create_location

Michael Barton
Yes. This is indeed what needs to be done with core.create_location.

I'm not even sure if it was working correctly before anyway. I don't see how it could. Any problem with me updating it like this?

Michael
____________________
C. Michael Barton
Director, Center for Social Dynamics & Complexity 
Professor of Anthropology, School of Human Evolution & Social Change
Arizona State University

voice:  480-965-6262 (SHESC), 480-727-9746 (CSDC)
fax:          480-965-7671 (SHESC),  480-727-0709 (CSDC)











On Oct 1, 2012, at 9:37 PM, Michael Barton <[hidden email]> wrote:

In the GRASS python library, core.create_location needs to be updated to use the new datum argument in g.proj.

I think I know how to update it, but it looks like there may be an error in the create_location code. It is using "datum" to refer to the "datumtrans" argument of g.proj. 

Currently, the code looks like this

def create_location(dbase, location,
                    epsg = None, proj4 = None, filename = None, wkt = None,
                    datum = None, desc = None):

...
    
    kwargs = dict()
    if datum:
        kwargs['datum'] = datum

I think it probably should be:

def create_location(dbase, location,
                    epsg = None, proj4 = None, filename = None, wkt = None,
                    datum = None, datumtrans=None, desc = None):

...
    
    kwargs = dict()
    if datum:
        kwargs['datum'] = datum
    if datumtrans:
        kwargs['datumtrans'] = datumtrans

The programmers manual would need to be updated to reflect this change. This means a change in the arguments for create_location. But it is currently sending the wrong argument to g.proj anyway. So this is also a bug fix to create_location.



Michael
____________________
C. Michael Barton
Director, Center for Social Dynamics & Complexity 
Professor of Anthropology, School of Human Evolution & Social Change
Arizona State University

voice:  480-965-6262 (SHESC), 480-727-9746 (CSDC)
fax:          480-965-7671 (SHESC),  480-727-0709 (CSDC)













_______________________________________________
grass-dev mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/grass-dev
Reply | Threaded
Open this post in threaded view
|

Re: need update to core.create_location

Michael Barton
Arrgg.

Before Paul's update to g.proj today, it turns out that g.proj would actually accept the argument "datum" as equivalent of the argument "datatrans", even though this is not in the manual. I don't know if that was intentional or an accident. So g.proj datumtrans=1 could also be expressed as g.proj datum=1. 

core.create_location is a python wrapper for g.proj in the GRASS python scripting library. So even though create_location incorrectly gave the data transform argument as "datum" instead of the correct "datatrans", it still worked when the datum argument was actually the data transform number (an integer). My guess is that this was an error but happened to work due to a lucky accident. However, it is actually documented in the programmers manual for core.create_location as: "datum datum transformation parameters (used for epsg and proj4)"

Now that g.proj has an 'official' argument of "datum" that takes the GRASS datum code, create_location works differently than it did before. In fact, it works 'correctly' as it relates to g.proj in that the "datum" argument now actually takes a datum code. However, it no longer has a way to set the datum transform. So we need to add a new argument of "datumtrans" to create_location. This change will be a big improvement to g.proj and the core.create_location wrapper for g.proj.

Hopefully this is understandable to those who are working with g.proj.

Note that this change in create_location, even though correcting an error, will break a script that uses core.create_location and assumes that the "datum" argument refers to the "datumtrans" value. It does not break a script that uses g.proj according to the manual.

Suggestions on how to proceed? 

If create_location is fixed, I can fix the location wizard. In fact, I've already done it locally but can't commit it until create_location is changed too.

Michael
____________________
C. Michael Barton
Director, Center for Social Dynamics & Complexity 
Professor of Anthropology, School of Human Evolution & Social Change
Arizona State University

voice:  480-965-6262 (SHESC), 480-727-9746 (CSDC)
fax:          480-965-7671 (SHESC),  480-727-0709 (CSDC)











On Oct 1, 2012, at 10:59 PM, Michael Barton <[hidden email]>
 wrote:

Yes. This is indeed what needs to be done with core.create_location.

I'm not even sure if it was working correctly before anyway. I don't see how it could. Any problem with me updating it like this?

Michael
____________________
C. Michael Barton
Director, Center for Social Dynamics & Complexity 
Professor of Anthropology, School of Human Evolution & Social Change
Arizona State University

voice:  480-965-6262 (SHESC), 480-727-9746 (CSDC)
fax:          480-965-7671 (SHESC),  480-727-0709 (CSDC)











On Oct 1, 2012, at 9:37 PM, Michael Barton <[hidden email]> wrote:

In the GRASS python library, core.create_location needs to be updated to use the new datum argument in g.proj.

I think I know how to update it, but it looks like there may be an error in the create_location code. It is using "datum" to refer to the "datumtrans" argument of g.proj. 

Currently, the code looks like this

def create_location(dbase, location,
                    epsg = None, proj4 = None, filename = None, wkt = None,
                    datum = None, desc = None):

...
    
    kwargs = dict()
    if datum:
        kwargs['datum'] = datum

I think it probably should be:

def create_location(dbase, location,
                    epsg = None, proj4 = None, filename = None, wkt = None,
                    datum = None, datumtrans=None, desc = None):

...
    
    kwargs = dict()
    if datum:
        kwargs['datum'] = datum
    if datumtrans:
        kwargs['datumtrans'] = datumtrans

The programmers manual would need to be updated to reflect this change. This means a change in the arguments for create_location. But it is currently sending the wrong argument to g.proj anyway. So this is also a bug fix to create_location.



Michael
____________________
C. Michael Barton
Director, Center for Social Dynamics & Complexity 
Professor of Anthropology, School of Human Evolution & Social Change
Arizona State University

voice:  480-965-6262 (SHESC), 480-727-9746 (CSDC)
fax:          480-965-7671 (SHESC),  480-727-0709 (CSDC)














_______________________________________________
grass-dev mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/grass-dev
Reply | Threaded
Open this post in threaded view
|

Re: need update to core.create_location

Paul Kelly
Michael Barton wrote:
> Arrgg.
>
> Before Paul's update to g.proj today, it turns out that g.proj would
> actually accept the argument "datum" as equivalent of the argument
> "datatrans", even though this is not in the manual. I don't know if that
> was intentional or an accident. So g.proj datumtrans=1 could also be
> expressed as g.proj datum=1.

The g.proj option has always been called "datumtrans", but GRASS allows
the user to shorten command-line options by dropping letters off the
end, as long as they still unambiguously identify the option (and GRASS
7 extends this a bit further). So beforehand as there was no "datum"
option, "datum" would have unambiguously identified "datumtrans". Now
you need to type at least "datumt" in order to distinguish it from the
new "datum" option.

I think as long as everywhere in GRASS 7 that might use the datumtrans=
option is updated to use it correctly, there should be no problem. Of
course we could also call the new option something other than "datum",
but it seems like such a clear and obvious name that it seems to me a
pity to have to change it!

One other thing - please don't backport for a few days; if I get time I
want to try and make an improvement to the code so that the ellipsoid
will always be changed if the datum is changed. As it is currently, it
is theoretically possible to change the datum to one that is
incompatible with the ellipsoid.

Paul
_______________________________________________
grass-dev mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/grass-dev
Reply | Threaded
Open this post in threaded view
|

Re: need update to core.create_location

Michael Barton
Thanks for the information Paul. This is a good tip to know for command-line typing. It is unfortunate, however, that this undocumented call was used in the Python scripting library. I agree with you that using "datum" for datums makes enormous sense. So hopefully we can get all straightened out.

I'll wait to do any committing until 1) you give me the goahead that you've fixed the issue in g.proj you mention and 2) I hear from Markus about updating create_location.

Thanks again for making such a fast fix for this.

Michael
______________________________
C. Michael Barton
Director, Center for Social Dynamics & Complexity
Professor of Anthropology, School of Human Evolution & Social Change
Arizona State University
Tempe, AZ  85287-2402
USA

voice: 480-965-6262 (SHESC), 480-727-9746 (CSDC)
fax:          480-965-7671(SHESC), 480-727-0709 (CSDC)
www: http://csdc.asu.edu, http://shesc.asu.edu
                http://www.public.asu.edu/~cmbarton

On Oct 2, 2012, at 1:40 AM, Paul Kelly <[hidden email]> wrote:

> Michael Barton wrote:
>> Arrgg.
>>
>> Before Paul's update to g.proj today, it turns out that g.proj would
>> actually accept the argument "datum" as equivalent of the argument
>> "datatrans", even though this is not in the manual. I don't know if that
>> was intentional or an accident. So g.proj datumtrans=1 could also be
>> expressed as g.proj datum=1.
>
> The g.proj option has always been called "datumtrans", but GRASS allows the user to shorten command-line options by dropping letters off the end, as long as they still unambiguously identify the option (and GRASS 7 extends this a bit further). So beforehand as there was no "datum" option, "datum" would have unambiguously identified "datumtrans". Now you need to type at least "datumt" in order to distinguish it from the new "datum" option.
>
> I think as long as everywhere in GRASS 7 that might use the datumtrans= option is updated to use it correctly, there should be no problem. Of course we could also call the new option something other than "datum", but it seems like such a clear and obvious name that it seems to me a pity to have to change it!
>
> One other thing - please don't backport for a few days; if I get time I want to try and make an improvement to the code so that the ellipsoid will always be changed if the datum is changed. As it is currently, it is theoretically possible to change the datum to one that is incompatible with the ellipsoid.
>
> Paul

_______________________________________________
grass-dev mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/grass-dev
Reply | Threaded
Open this post in threaded view
|

Re: need update to core.create_location

Paul Kelly
Hi Michael,

Michael Barton wrote:
> Thanks for the information Paul. This is a good tip to know for command-line typing. It is unfortunate, however, that this undocumented call was used in the Python scripting library. I agree with you that using "datum" for datums makes enormous sense. So hopefully we can get all straightened out.
>
> I'll wait to do any committing until 1) you give me the goahead that you've fixed the issue in g.proj you mention and 2) I hear from Markus about updating create_location.

I've fixed the issue with g.proj now in r53305. It will now always
update the ellipsoid when overriding the datum. Hopefully that should be
all you need.

Paul
_______________________________________________
grass-dev mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/grass-dev
Reply | Threaded
Open this post in threaded view
|

Re: need update to core.create_location

Glynn Clements
In reply to this post by Paul Kelly

Paul Kelly wrote:

> > Before Paul's update to g.proj today, it turns out that g.proj would
> > actually accept the argument "datum" as equivalent of the argument
> > "datatrans", even though this is not in the manual. I don't know if that
> > was intentional or an accident. So g.proj datumtrans=1 could also be
> > expressed as g.proj datum=1.
>
> The g.proj option has always been called "datumtrans", but GRASS allows
> the user to shorten command-line options by dropping letters off the
> end, as long as they still unambiguously identify the option (and GRASS
> 7 extends this a bit further). So beforehand as there was no "datum"
> option, "datum" would have unambiguously identified "datumtrans". Now
> you need to type at least "datumt" in order to distinguish it from the
> new "datum" option.
>
> I think as long as everywhere in GRASS 7 that might use the datumtrans=
> option is updated to use it correctly, there should be no problem. Of
> course we could also call the new option something other than "datum",
> but it seems like such a clear and obvious name that it seems to me a
> pity to have to change it!

Consider whether the existing option should be renamed to datum_trans=.
The abbreviation rules in 7.0 should accept datumtrans= as an
abbreviation, but will also accept e.g. dt= or dtrans=.

More generally, if one option is a prefix of another, adding an
underscore to the longer option will typically result in the minimum
abbreviation being substantially shorter.

--
Glynn Clements <[hidden email]>
_______________________________________________
grass-dev mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/grass-dev
Reply | Threaded
Open this post in threaded view
|

Re: need update to core.create_location

Paul Kelly
Glynn Clements wrote:
>
> Consider whether the existing option should be renamed to datum_trans=.
> The abbreviation rules in 7.0 should accept datumtrans= as an
> abbreviation, but will also accept e.g. dt= or dtrans=.

Sounds like a good idea to me. Since we're breaking compatibility with
the scripts that used datum= instead of datumtrans= so that they will
need updated anyway, it seems like a good time to make this further
change to make things neater.

Paul
_______________________________________________
grass-dev mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/grass-dev
Reply | Threaded
Open this post in threaded view
|

Re: need update to core.create_location

Michael Barton
I agree. This makes sense and is a less confusing shortcut.

Michael
____________________
C. Michael Barton
Director, Center for Social Dynamics & Complexity
Professor of Anthropology, School of Human Evolution & Social Change
Arizona State University

voice: 480-965-6262 (SHESC), 480-727-9746 (CSDC)
fax:          480-965-7671 (SHESC),  480-727-0709 (CSDC)
www: http://www.public.asu.edu/~cmbarton, http://csdc.asu.edu











On Oct 3, 2012, at 6:14 AM, Paul Kelly <[hidden email]>
 wrote:

> Glynn Clements wrote:
>>
>> Consider whether the existing option should be renamed to datum_trans=.
>> The abbreviation rules in 7.0 should accept datumtrans= as an
>> abbreviation, but will also accept e.g. dt= or dtrans=.
>
> Sounds like a good idea to me. Since we're breaking compatibility with the scripts that used datum= instead of datumtrans= so that they will need updated anyway, it seems like a good time to make this further change to make things neater.
>
> Paul

_______________________________________________
grass-dev mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/grass-dev
Reply | Threaded
Open this post in threaded view
|

Re: need update to core.create_location

Paul Kelly
Michael Barton wrote:
 > On Oct 3, 2012, at 6:14 AM, Paul Kelly <[hidden email]>
 >  wrote:
 >
 >> Glynn Clements wrote:
 >>> Consider whether the existing option should be renamed to datum_trans=.
 >>> The abbreviation rules in 7.0 should accept datumtrans= as an
 >>> abbreviation, but will also accept e.g. dt= or dtrans=.
 >> Sounds like a good idea to me. Since we're breaking compatibility
with the scripts that used datum= instead of datumtrans= so that they
will need updated anyway, it seems like a good time to make this further
change to make things neater.
 >
 > I agree. This makes sense and is a less confusing shortcut.

OK - change committed now; the option is now called datum_trans=. I
guess we should be careful to avoid backporting this option name change
to 6.x, rather only backporting the addition of the new datum= option? I
will leave that decision to someone else anyway.

Paul


_______________________________________________
grass-dev mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/grass-dev