search

cheminfo-js / spectra-data

Library to deal with spectra (IR, UV, ...), 1D NMR as well as 2D NMR
https://cheminfo-js.github.io/spectra-data
MIT License
4 stars 0 forks source link

SD.fromJcamp should return a promise #25

Closed lpatiny closed 7 years ago

lpatiny commented 8 years ago

We should allow a url instead of a textfile containing the jcamp so it should return a Promise.

You may get inspired by image-js

https://github.com/image-js/core/blob/d166bd5a0e6b493097c9feefd1ca7bf3903052c4/src/image/load.js#L24

targos commented 8 years ago

I don't agree with this. There are many ways to load a JCAMP in the browser and Node.js and it is not the job of this library to do it for you

lpatiny commented 8 years ago

I don't really understand what you don't agree with ? The idea is that we can easily create an instance of the SD object using method from SD.fromJcamp, SD.fromBruker, SD.fromArray, ... All those methods will call the corresponding libraries and ensure it is converted to the format that SD understands.

targos commented 8 years ago

I don't agree with should return a Promise. I think it should accept a JCAMP string and return an instance of SD.

lpatiny commented 8 years ago

OK so we should rather also have SD.fromJcampURL() ?

targos commented 8 years ago

I you really want it then yes, but I think the download should not be handled by this library

lpatiny commented 8 years ago

OK so we only load the jcamp and it is sync ! So it returns directly the spectraData instance.

andcastillo commented 8 years ago

That is more or less what I conclude. I have a new set of function to load the spectraData from url for example, but it just call the sync spectra-data library. The spectra-creator is not related to the functionality of sd..