-
Notifications
You must be signed in to change notification settings - Fork 14
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
ISSUE #134 Support ECOStress Spectral Library #153
Conversation
Merge pull request #152 from bdegley4789/master
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking this one @bdegley4789 please consider my comments.
examples/example_ecostress.py
Outdated
dir[:] = [d for d in dir] | ||
for items in fnmatch.filter(files, "*spectrum.txt"): | ||
#Remove vegation files because their format causes errors in the Spectral class | ||
if "vegetation" in items: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you remind me again why we are not using the vegetation indexes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On lines 4 & 5 Vegetation Spectra has Genus and Species instead of Subclass and Particle Size entries like other Spectra.
The ASTER conversion function throws an error when it reads lines 4 & 5 so I just removed the vegetation files.
If we wanted to read Vegetation files then we would need to change lines 4 & 5 to give them a subclass and particle size with a default value like I did with Spectral 7 conversion function. I know you said you prefer if we not plug in default values. Let me know if you want me to change this, it wouldn't be very hard
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
manmade.concrete.constructionconcrete.solid.all.0598uuucnc.jhu.becknic.spectrum.txt
Name: Construction Concrete
Type: manmade
Class: Concrete
Subclass: Construction Concrete
Particle Size: Solid
Sample No.: 0598UUUCNC
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nonphotosyntheticvegetation.bark.abies.concolor.all.vh311.ucsb.asdnicolet.spectrum.txt
Name: Abies concolor bark
Type: non photosynthetic vegetation
Class: bark
Genus: Abies
Species: concolor
Sample No.: VH311
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, ideally we want to be able to use every spectra from the entire ECOSTRESS library. If you could implement the suggested workaround for including the vegetation files it would be appreciated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lewismc Ok, I fixed it so it works with Vegetation files. I added the Subclass and Particle Size and removed the Genus and Species. It can now convert the entire EcoStress library
input_file.write(contents) | ||
input_file.close() | ||
|
||
ecoStress = mineral.AsterConversion() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please change ecoStress
to ecostress
, thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once you address this then we can think about ripping this code out of the example and embedding it in conversion.py
under a new Class the same as the other conversion functions.
examples/example_ecostress.py
Outdated
files of `ECOStress Spectral Library <https://speclib.jpl.nasa.gov/>`_ | ||
|
||
Dependencies | ||
`ECOStress Spectral Library <https://speclib.jpl.nasa.gov/>`_ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should be more descriptive here.
The downloads page includes the following artifacts
All Lunar (17 files)
All Man-made (72 files)
All Meteorites (59 files)
All Minerals (3104 files)
All Non-Photosynthetic Vegetation (62 files)
All Rock (647 files)
All Soil (120 files)
All Vegetation (1031 files)
All Water (9 files)
Or
ALL Spectra (5104 files)
Which of the above do we download?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ALL Spectra, I have now made a comment in the code to specify
# Floor, Boston, MA 02110-1301, USA. | ||
|
||
Mineral Classification API | ||
********************************* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to add the following
.. autoclass:: pycoal.mineral.SpectalToAsterFileFormat
:special-members:
:members:
.. autoclass:: pycoal.mineral.FullSpectralLibrary7Convert
:special-members:
:members:
Hi @bdegley4789 I pushed some improvements, when you have time, please resolve the conflict. Sorry about that. |
All good, it's resolved. I'm not sure what it was. It looks like it was the same thing I added to mineral.rst |
I'm not sure why the travis ci failed on python 3.3 but passed on all the others. It looks like the commit I did for EcoStress passed though since it has the green check. Must be something with the merge |
docs/source/mineral.rst
Outdated
.. autoclass:: pycoal.mineral.AsterConversion | ||
:special-members: | ||
:members: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can now be removed as the classes are present within thr new conversion.py
file.
@lewismc I think this code addresses #134 .
The EcoStress Spectral Library is almost the same format as ASTER. To make it the same as ASTER you just need to add 5 ‘\n’ after the description to every .spectrum.txt file. We can extract this into its own class and write test cases later. For now I just have it as an example script to demo.
Downloading EcoStress can take awhile for your request to go through.
If you want to run on your own system download and unzip the zip I staged in here to pycoal/examples then just run.
python example_ecoStress.py ecospeclib-1525266883141