Requested Changes to Spectral Library Version 7 issue #141
Conversation
bdegley4789
changed the title
|
I'll try looking into why docker and travis ci aren't passing. I think travis-ci has been down a while, but Docker should be passing. Let me know what you think about the rest of the code @lewismc |
|
@bdegley4789 thanks for pushing the PR. |
|
@lewismc testing all of this and viewing results will be the next part of solving issues #105 but if you run with the cli you would just need to pass the USGS Spectral Library v7 convolved envi .hdr file as the second argument If you instead ran with the example_mineral you would need to change the path location or move the file to the current location and change the name to s07av95a_envi.hdr pycoal/examples/example_mineral.py Line 64 in 8ca63f1 For mining.py we will need to change the 3 spectra they are looking for to be the equivalent names for the spectra in USGS Spectral Library v7 Line 24 in 8ca63f1 It doesn’t look like anything needs to be done for environment.py |
There was a problem hiding this comment.
@bdegley4789 this is a bit better but there are still several major issues with the implementation. This requires much more work so please begin incrementally updating base upon the comments I've made.
| @@ -0,0 +1,106 @@ | |||
| #Copyright (C) 2017-2018 COAL Developers | |||
There was a problem hiding this comment.
@bdegley4789 there are a few major issues with the code in this file
- The documentation does not mention that you need to have the file
./ASCIIdata/ASCIIdata_splib07a/ChapterA_ArtificialMaterials/splib07a_Alizarin_crimson_(dk)_GDS780_ASDFRa_AREF.txtpresent in the current working directory in order for the conversion to work correctly... this would need to be corrected. - The logic is intrinsically linked to the man made compound Alizarin_crimson... which is a paint pigment. There is absolutely no rational justification for us having an example of a conversion and spectral classification based upon a paint pigment as it has nothing to do with the COAL project. We are looking for (at a bare minimum) examples based on elements associated with COAL and surface mining activities e.g. Renyolds_Tnl_Sludge, Renyolds_TnlSldgWet and Schwertmannite. Make sure you ensure that the examples are related to the above. These proxy classes are extracted directly from mining.py. I've referenced these several times in the past so please take the guidance on board. Thanks.
- The code in this file is a complete copy of the proposed code you've implemented in pycoal/mineral.py... why are you duplicating the code? You should just be using the
SpectralToAsterConversion.convert(...)function you created in pycoal/mineral.py.
|
|
||
| ''' | ||
| splib07_convolved -- a script which will generate envi .sli and .hdr convolved library | ||
| files of USGS Spectral Library Version 7 <https://speclab.cr.usgs.gov/spectral-lib.html> |
| files of USGS Spectral Library Version 7 <https://speclab.cr.usgs.gov/spectral-lib.html> | ||
|
|
||
| Dependencies | ||
| USGS Spectral Library Version 7 <https://speclab.cr.usgs.gov/spectral-lib.html> |
|
|
||
| This code has three parts. | ||
| First, it loops through the USGS Spectral Library version 7 | ||
| <https://speclab.cr.usgs.gov/spectral-lib.html> and moves all spectra files to a |
| if not os.path.exists(directory): | ||
| os.makedirs(directory) | ||
|
|
||
| exclude = set(['usgs_splib07_modified']) |
There was a problem hiding this comment.
We should be looking only in the ./ASCIIdata/ASCIIdata_* wildcard directories. It is up to you how you wish to implement this.
| # Convert all files | ||
| files = os.listdir(library_filename) | ||
| for x in range(0, len(files)): | ||
| name = 'usgs_splib07_modified/' + files[x] |
There was a problem hiding this comment.
Why not reuse one of the string variables you've created above?
|
|
||
| #This will generate three files s07av95a_envi.hdr, s07av95a_envi.hdr.sli,splib.db and dataSplib07.db | ||
| #For a library in ASTER Library Version 2.0 <https://asterweb.jpl.nasa.gov/> format | ||
| library_filename = 'usgs_splib07_modified' |
There was a problem hiding this comment.
Why create yet another sting variable with the same value...?
| This class provides a method for converting the `ASTER Spectral | ||
| Library Version 2.0 <https://asterweb.jpl.nasa.gov/>`_ into ENVI format. | ||
| This class provides a method for converting the ASTER Spectral | ||
| Library Version 2.0 <https://asterweb.jpl.nasa.gov/> into ENVI format. |
There was a problem hiding this comment.
Why remove the hyperlink? revert this...
| def __init__(self): | ||
| """ | ||
| This class provides a method for converting USGS Spectral Library Version 7 | ||
| <https://speclab.cr.usgs.gov/spectral-lib.html> .txt files into ASTER Spectral |
| pass | ||
|
|
||
| @classmethod | ||
| def convert(cls, library_filename=""): |
There was a problem hiding this comment.
This code is duplicate as that from examples/convert_to_aster.py. If also suffers from the same issues I've pointed out in my review of that file... correct and resubmit a PR.
|
@lewismc I think there is an issue with the bands that need adjustment but I think I almost have it working with this method. I’ll keep you updated on my progress. |
Okay, I made the recommended changes for issue #105 I fixed the variable names, headers, added more comments where needed and now this can all be done via the command line since I got os.walk to work like we discussed on my last pull request.
I also just combined all the files I had created into one python script and left convert_to_aster.py if we want to test this method on another spectral library in the future. All the users needs to do now
It should take about 10-15 minutes to run and it will create the envi .sli and .hdr files in the examples directory with the spectra names and wavelength bands present in the hdr file