Add ASCII file to geonode. #232

ismailsunni · 2017-05-31T08:40:39Z

Partial fix for #159

Current state:

We can't upload asc file
We can upload tif file

This PR will make geonode able to upload asc file. We need to enable it first before we can download the asc file.

Gustry

LGTM.

Maybe we should merge this commit straight to geonode upstream, no? Because it's not related to QGIS Server

Gustry · 2017-05-31T08:48:52Z

geonode/layers/forms.py

@@ -134,7 +134,8 @@ def clean(self):
            if cleaned["xml_file"] is not None:
                xml_file = cleaned["xml_file"].name

-        if not cleaned["metadata_upload_form"] and base_ext.lower() not in (".shp", ".tif", ".tiff", ".geotif", ".geotiff"):
+        if not cleaned["metadata_upload_form"] and base_ext.lower() not in (
+                ".shp", ".tif", ".tiff", ".geotif", ".geotiff", ".asc"):
            raise forms.ValidationError(
                "Only Shapefiles and GeoTiffs are supported. You uploaded a %s file" %


You need ASC here too

ismailsunni · 2017-05-31T09:05:57Z

@Gustry I also have the same thought actually. May be after I finished the download ascii file, I will make a PR to upstream.
I have asked them in the gitter channel actually, about asc file. May be they have a specific reason to not support it.

codecov · 2017-05-31T09:13:46Z

Codecov Report

Merging #232 into master-qgis_server will decrease coverage by 0.04%.
The diff coverage is 34.21%.

@@                  Coverage Diff                   @@
##           master-qgis_server     #232      +/-   ##
======================================================
- Coverage               44.28%   44.23%   -0.05%     
======================================================
  Files                     254      254              
  Lines                   18292    18323      +31     
  Branches                 2409     2414       +5     
======================================================
+ Hits                     8100     8106       +6     
- Misses                   9719     9742      +23     
- Partials                  473      475       +2

Impacted Files	Coverage Δ
geonode/qgis_server/urls.py	`100% <ø> (ø)`	⬆️
geonode/upload/files.py	`84% <ø> (ø)`	⬆️
geonode/upload/views.py	`20% <ø> (ø)`	⬆️
geonode/settings.py	`90.61% <ø> (ø)`	⬆️
geonode/qgis_server/signals.py	`71.01% <0%> (-6.26%)`	⬇️
geonode/layers/models.py	`77.95% <100%> (ø)`	⬆️
geonode/layers/forms.py	`85.91% <100%> (ø)`	⬆️
geonode/qgis_server/models.py	`81.08% <100%> (+0.52%)`	⬆️
geonode/layers/tests.py	`97.49% <100%> (+0.03%)`	⬆️
geonode/qgis_server/views.py	`12.34% <6.25%> (-0.43%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b2c8171...587c0e8. Read the comment docs.

lucernae

LGTM

We might also need to create proper link in Geoserver and QGIS Server to make it a downloadable.

gubuntu · 2017-06-01T07:12:37Z

nice work. yes it does need to be downloadable too

ismailsunni · 2017-06-02T03:50:11Z

The links are added now. I do not provide ASCII link for GeoTIFF and vice versa.

I think I will make a PR to geonode upstream first.

ismailsunni · 2017-06-02T04:06:02Z

I made a PR to geonode GeoNode#3105

Since we made a big change in qgis_server app, I only submitted the geoserver one.

Gustry · 2017-06-02T05:47:18Z

geonode/qgis_server/views.py

+    except ObjectDoesNotExist:
+        msg = 'No QGIS Server Layer for existing layer %s' % layername
+        logger.debug(msg)
+        return Http404(msg)


can we use 'get_object_or404' ?

Gustry · 2017-06-02T06:25:09Z

I just noticed your GIF.
Shouldn't we get a file back instead of a string?

(IMHO, the user clicked on "download", so I expect a file)

Gustry · 2017-06-02T06:27:01Z

Also, I noticed the QGIS plugin supports only asc, not ASC or whatever.
https://github.com/kartoza/otf-project/blob/master/filters/map_composition.py#L114

Does your code change the extension if it's 'aSc' ?
I can also improve the plugin ...

ismailsunni · 2017-06-02T08:42:08Z

@Gustry
Yes, it's a text file. I use the same approach as the other links (pointing to the file directly). For asc, I can't use content-type image/asc. It will give download a ac file, without extension. Then, I use text/asc and it give similar behavior as the other links.

Gustry · 2017-06-02T09:11:20Z

If you want to make it downloadable, you need to set the Content-Disposition in the HttpResponse, like we are doing for the zip.

BUT, I'm just thinking that it's a not a so good idea to make downloadable an ASC file as we are losing the PRJ file. An ASC file without projection is bad. Maybe we should provide a ZIP file, like we are already doing for shapefile.

IMHO, I would not like to have an ASC file, without the projection downloaded automatically. It's like missing an important part of the dataset. @gubuntu any thought about this? Is-it an issue?

gubuntu · 2017-06-02T09:23:07Z

Yes I think a zip file incl prj is good and necessary. What about including the metadata xml in the zip as well (for all downloads)?

ismailsunni · 2017-06-02T09:25:12Z

Also, I noticed the QGIS plugin supports only asc, not ASC or whatever.
https://github.com/kartoza/otf-project/blob/master/filters/map_composition.py#L114

Does your code change the extension if it's 'aSc' ?
I can also improve the plugin ...

No. It only handles asc. Should we handle other cases also? I think ASC is fine, but the other combination seems strange.

If you want to make it downloadable, you need to set the Content-Disposition in the HttpResponse, like we are doing for the zip.

BUT, I'm just thinking that it's a not a so good idea to make downloadable an ASC file as we are losing the PRJ file. An ASC file without projection is bad. Maybe we should provide a ZIP file, like we are already doing for shapefile.

IMHO, I would not like to have an ASC file, without the projection downloaded automatically. It's like missing an important part of the dataset. @gubuntu any thought about this? Is-it an issue?

Yeah, I agree on the missing projection file. The tiff is also missing the prj file.

Actually, we have already had download zip file link, but it's just not exposed to the user.

I think I will expose the zip file link. Then, for the asc file, should I do the same as zip file, or keep using the txt/asc (to keep the behaviour uniform)?

cc @gubuntu

Gustry · 2017-06-02T09:43:33Z

No. It only handles asc. Should we handle other cases also? I think ASC is fine, but the other combination seems strange.

Sorry Ismail, I didn't understand. I'm not sure the plugin will work if we upload a .ASC. (just in case)

The ZIP link will take care of whatever the format is (geotiff, asc, ...) so I think it's fine. Let's use only view. No ?

ismailsunni · 2017-06-02T09:49:48Z

Sorry Ismail, I didn't understand. I'm not sure the plugin will work if we upload a .ASC. (just in case)

I mean, the layer upload page, can handle different font case, e.g. asc, ASC or aSc. But the qgis server django app code only handles asc.

Should we just make the extestion lower case, and you don't need to change anything in the otf code?

The ZIP link will take care of whatever the format is (geotiff, asc, ...) so I think it's fine. Let's use only view. No ?

Yes. The zip can handle any format. Ok, I will keep the asc donwload link like now.

ismailsunni · 2017-06-03T03:09:45Z

Hi @gubuntu and @Gustry
I will merge this one first, and add the ZIP files download links in the different PR. For solving asc support, this PR is already working.

I just want to avoid conflict after @lucernae rebase from geonode master.

Enable ASCII file upload and create link for ASCII file.

ismailsunni requested review from gubuntu and lucernae May 31, 2017 08:40

ismailsunni added the in progress label May 31, 2017

Gustry approved these changes May 31, 2017

View reviewed changes

lucernae approved these changes Jun 1, 2017

View reviewed changes

ismailsunni force-pushed the ascii_file_master branch from 687d6aa to 35ec41e Compare June 2, 2017 03:48

Gustry reviewed Jun 2, 2017

View reviewed changes

ismailsunni force-pushed the ascii_file_master branch from 79020c2 to c72ebdc Compare June 2, 2017 08:45

ismailsunni added 6 commits June 2, 2017 16:51

Enable ascii file upload.

5785927

Unit test for ascii file.

e24a79d

Add ASCIIs to the error message.

6a09327

Add download link for ASCII (text file).

c748a2e

PEP8.

2b26a79

Use get_object_or_404.

587c0e8

ismailsunni force-pushed the ascii_file_master branch from c72ebdc to 587c0e8 Compare June 2, 2017 09:56

ismailsunni merged commit a7ec2f2 into kartoza:master-qgis_server Jun 3, 2017

ismailsunni removed the in progress label Jun 3, 2017

This was referenced Jun 3, 2017

download original raster #159

Closed

Add download zipped files for non-shapefile. #239

Merged

lucernae referenced this pull request in lucernae/geonode Jun 24, 2017

Add ASCII file to geonode. (#232)

cb1dfc9

Enable ASCII file upload and create link for ASCII file.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ASCII file to geonode. #232

Add ASCII file to geonode. #232

ismailsunni commented May 31, 2017

Gustry left a comment

Gustry May 31, 2017

ismailsunni commented May 31, 2017

codecov bot commented May 31, 2017 •

edited

Loading

lucernae left a comment

gubuntu commented Jun 1, 2017

ismailsunni commented Jun 2, 2017

ismailsunni commented Jun 2, 2017 •

edited

Loading

Gustry Jun 2, 2017

Gustry commented Jun 2, 2017

Gustry commented Jun 2, 2017

ismailsunni commented Jun 2, 2017

Gustry commented Jun 2, 2017

gubuntu commented Jun 2, 2017 •

edited

Loading

ismailsunni commented Jun 2, 2017

Gustry commented Jun 2, 2017

ismailsunni commented Jun 2, 2017

ismailsunni commented Jun 3, 2017

Add ASCII file to geonode. #232

Add ASCII file to geonode. #232

Conversation

ismailsunni commented May 31, 2017

Gustry left a comment

Choose a reason for hiding this comment

Gustry May 31, 2017

Choose a reason for hiding this comment

ismailsunni commented May 31, 2017

codecov bot commented May 31, 2017 • edited Loading

Codecov Report

lucernae left a comment

Choose a reason for hiding this comment

gubuntu commented Jun 1, 2017

ismailsunni commented Jun 2, 2017

ismailsunni commented Jun 2, 2017 • edited Loading

Gustry Jun 2, 2017

Choose a reason for hiding this comment

Gustry commented Jun 2, 2017

Gustry commented Jun 2, 2017

ismailsunni commented Jun 2, 2017

Gustry commented Jun 2, 2017

gubuntu commented Jun 2, 2017 • edited Loading

ismailsunni commented Jun 2, 2017

Gustry commented Jun 2, 2017

ismailsunni commented Jun 2, 2017

ismailsunni commented Jun 3, 2017

codecov bot commented May 31, 2017 •

edited

Loading

ismailsunni commented Jun 2, 2017 •

edited

Loading

gubuntu commented Jun 2, 2017 •

edited

Loading