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

Adding crs_converter to Galaxy ecology #119

Open
wants to merge 22 commits into
base: master
Choose a base branch
from

Conversation

Rassine
Copy link
Contributor

@Rassine Rassine commented Jul 15, 2024

CRS Converter transform coordinate reference systems of shapefiles using the st_transform function, the tool support a limited selection of interesting CRS, adding a new CRS can be done by simply adding a new condition inside the xml file and re-using or creating parrameters inside the marco file. The Cheetah script generates the PROJ string and handles special cases, this way I avoided having hundreds of lines of codes for each conditional statement and adding a new CRS does not require any modification of the cheetah command most of the time.
The output of the tool can eather be a shapefile, a pdf or an image.

Copy link
Collaborator

@Marie59 Marie59 left a comment

Choose a reason for hiding this comment

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

Really nice tool and quite an impressive xml !!;)
I tried to review what I could (I may double check later on)
I think the cheetah par may need some reviewing but I am afraid to make mistakes there ;)

PS : for the white spaces I did not correct all of them
Great work @Rassine !!

tools/crs_converter/CRSconverter.xml Outdated Show resolved Hide resolved
tools/crs_converter/CRSconverter.xml Outdated Show resolved Hide resolved
tools/crs_converter/CRSconverter.xml Outdated Show resolved Hide resolved
tools/crs_converter/CRSconverter.xml Outdated Show resolved Hide resolved
tools/crs_converter/CRSconverter.xml Outdated Show resolved Hide resolved
tools/crs_converter/CRS_macros.xml Outdated Show resolved Hide resolved

<xml name='Y_options'>
<param name ='y_0' type='integer' value='0'
label='Y' help='False northing, northing at false origin or northing at projection centre
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't know much about projection and I don't understand this help part. Maybe it could be clearer for non expert

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree with you, however these descriptions are directly from the PROJ.org website, I'm not certain I can provide a bether description, I will try but I'm not expert myself!


<xml name='X_options'>
<param name ='x_0' type='integer' value='0'
label='X' help='False easting, easting at false origin or easting at projection centre
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here for the help I don't understand much


<xml name='longitude_options'>
<param name ='lon_0' type='integer' value='0'
label='Longitude' help='Central meridian/longitude of natural origin,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same the help is not clear for me

<xml name='ellps_options'>
<param name ='ellps' type='select' checked='true'
label='Ellipsoid and Datum' help='The name of built-in ellipsoid definition.
See https://proj.org/en/9.4/usage/ellipsoids.html#ellipsoids for more informations.'>
Copy link
Collaborator

Choose a reason for hiding this comment

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

more information in the help section ?

Rassine and others added 7 commits July 30, 2024 08:57
@Rassine
Copy link
Contributor Author

Rassine commented Jul 30, 2024

@Marie59 Thanks again for your review, I tried to put more information on the help parts you mentionend, but I belive with mixed succes, these description are very context dependent, looking directly at the projection on the PROJ website seems to be the easyest way to undersand the descriptions, I added direct links for each featured projection.

Copy link
Collaborator

@Marie59 Marie59 left a comment

Choose a reason for hiding this comment

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

I think it's better with your new help section and clearer !

tools/crs_converter/CRSconverter.xml Outdated Show resolved Hide resolved
tools/crs_converter/CRSconverter.xml Outdated Show resolved Hide resolved
tools/crs_converter/CRSconverter.xml Outdated Show resolved Hide resolved
https://proj.org/en/9.4/usage/projections.html
https://proj.org/en/9.4/geodesic.html

https://proj.org/en/9.4/operations/projections/index.html
Copy link
Collaborator

Choose a reason for hiding this comment

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

And same here put as I did before this way the user just have to click on the sentence and it open the page directly :)

tools/crs_converter/CRSconverter.xml Outdated Show resolved Hide resolved
'$__tool_directory__/CRSconverter.R'
$os.path.join($input_file.extra_files_path, 'shapefile.shp')
'$output_file_format'
#for $param_name in $input_param.keys() ## this loop successively calls all the names and values of the PROJ string for each option of the conditional, it avoids a 100 lines command.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you maybe add a comment here how the commandline in the end should look like?
I guess something like + lat_ts + south ...?

#if $param_name=='south' and $input_param[$param_name]=='false'
#pass
#elif $param_name=='south' and $input_param[$param_name]=='true'
' ' '+' $param_name
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
' ' '+' $param_name
' + ' $param_name

Would that also work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it would be : ' +' $param_name parameters and '+' are seperatated with others elements but not between themselves, blank spaces and '+' are separated because the R script needs to ignore the 1st blank space given as argument to create a proper PROJ string. this would work:

#if $param_name!='__current_case__'   
    #if $param_name=='south' and $input_param[$param_name]=='false'  
        #pass  
    #elif $param_name=='south' and $input_param[$param_name]=='true'
        ' +' $param_name
    #elif $param_name=='approx' and $input_param[$param_name]=='false'
        #pass
    #elif $param_name=='approx' and $input_param[$param_name]=='true'
        ' +' $param_name
    #elif $param_name=='lat_ts' and $input_param[$param_name]=='0'
        #pass
    #else
        ' ' '+' $param_name '='
        $input_param[$param_name]
#end if 

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.

4 participants