capstone-coal / capstone-coal.github.io

COAL Project Website
https://capstone-coal.github.io/
Apache License 2.0
3 stars 1 forks source link

Follow Up on Improving GDAL/QGIS Installation Instructions #45

Closed ghost closed 6 years ago

ghost commented 6 years ago

This issue is to look into improving the GDAL/QGIS installation instructions on the website as discussed in the originally misfiled pycoal issue 116. Commit 4504f67 was reverted by commit a980eff because the official version of QGIS is currently out of date. Recommend leaving unassigned until either the official version of QGIS can be verified to incorporate the rotation patch or a cross-platform installation procedure can be devised.

bdegley4789 commented 6 years ago

Thanks for moving this issue to the website @browtayl We discussed this issue in our meeting today. I plan to keep working on it. It seems like it is just something I have done wrong on my system, probably an issue with my path, installing an old version of gdal in frameworks or building an old version of gdal. I will update the instructions with a working solution as soon as I find one

ghost commented 6 years ago

Oh cool, then I'll set you as the assignee and let you take over. Dealing with all of these libraries can be a real pain, so I'm not surprised there have been issues getting it to work. The original instructions were biased towards GNU/Linux because the majority of last year's team used Debian or Ubuntu and supporting multiple platforms was not part of our requirements.

If patching GDAL and QGIS becomes a regular problem—quite possible when extending a free software library to support a proprietary image format—then it would be tempting to just build something that can be redistributed rather than have end-users compile stuff. But that would be a separate issue.

bdegley4789 commented 6 years ago

I fixed all the trace errors with gdal I had when running environmental correlation in the Alpha release video. As you can see in the photos environmental correlation runs without any trace errors or warnings. And the FlowLines are now clearly visible. In pull request #47 I updated the original installation instructions with what fixed the problems on my system. environment_run environmental_correlation_qgis

bdegley4789 commented 6 years ago

I still think we should refer users to the latest version of QGIS and GDAL on their websites. I guess with GDAL we could have it be optional to get the 2.2 branch to fix the rotational error. However, @browtayl it looks like the Windows version of QGIS displays the image correctly rotated, as it is displayed in the posters but the mac OS X version of QGIS which I used in the demo video seems to always display the QGIS images vertically. I am using both of these at 2.18 QGIS and 2.2.3 GDAL. I made a note about this for mac users in my pull request for updating QGIS and GDAL installation instructions.

bdegley4789 commented 6 years ago

On Windows qgis 2.18 windows_qgis

bdegley4789 commented 6 years ago

On Mac qgis 2.18 mac_qgis

ghost commented 6 years ago

That's a really interesting finding, somewhat unexpected. The QGIS website does not provide very much detail on what versions they use to build the software, so this process of trial and error is genuinely useful.

I would agree this makes a stronger case for linking to the official release, maybe with the caveat that the Mac and maybe GNU/Linux versions haven't caught up with the rotation fix yet as noted in your pull request.

The rotation fix is relatively cosmetic in the QGIS use case because it only affects how the data is displayed, though it becomes important when looking at more than one flightline or data set. It probably shouldn't be optional when installing GDAL for processing because in that case the rotation header doesn't get set correctly and then the environmental correlation breaks.

Let's see if @lewismc has any comments on this and move forward with integrating the changes. We can leave specific code review in pull https://github.com/capstone-coal/capstone-coal.github.io/pull/47. In the meantime thanks for the detailed review of this issue!

lewismc commented 6 years ago

If what has been committed in https://github.com/capstone-coal/capstone-coal.github.io/pull/49 is good then I am happy to progress. This has been a PITA, but I am glad that we are by it now and can move on to actually improving the toolkit again. Thanks.