-
Notifications
You must be signed in to change notification settings - Fork 2
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
Suggested first steps #1
Comments
As first step the documentation of matchms: And the GitHub repo: |
A first required function would be the conversion of R |
My second thought is how we want to make comparison function from |
Maybe we split some more detailed discussion into different issues (e.g. #3 for the |
The second question is a good one. Maybe we could first try to convert In a second approach we could try the other way round, do all in R but calling the spectra similarity function from |
Interfacing matchms this way is definitely possible, but with the MSP backend, these two packages should already be compatible in their native environments. |
@hechth, yes this would also be possible, but inconvenient for the R programmer, and when your using a complete workflow in R. As well as msmatch does not store the metadata so efficiently as in a spectra object. But it would be a convenient way for me if I want to compare a large dataset. My idea would be to have it in the |
The problem is that |
No objections against a different function - but IMHO the loop and comparising should all be performed in Python (if possible). The worst would be to have a Better in my opinion: convert a whole |
I think we can write a nice wrapper around |
I would suggest the following:
The first use case could thus be to have an R function that 1) converts the |
I will add the documentation and subselection of the metadata to it. |
I would suggest for simplicity to have one phyton script that is executed with |
We can check, what is faster: Running it with reticulate or directly in Python. I will change then the return of the conversion directly to a Python object, then we don't have to think about including |
Agree - let's have both ways. |
@michaelwitting if you mean |
The first functionality to convert between Python and R is now implemented in PR #10 (extending the functions from @michaelwitting a bit). For the whole package functionality I would suggest is the following:
would that be OK for you @michaelwitting @chufz @hechth ? |
I think this makes sense, the wrapper function will just call the conversion and score computation, but having both in the API is reasonable I think. |
I agree, seems like the easiest way for "R people". |
For the wrapper function I would suggest to go a similar way than with the I can take care of this side of the code if you guys/girls (maybe you have that already @chufz ?) come up with python function(s) to run the comparison and collect the results. |
Yes, I think the package should allow to use the python functionality in a way similar to the current |
I think we should have one function similar to |
I can have a look into it tomorrow, I am just not that fast... |
@chufz, No worries. Ideally, we would need a python function or script that takes two list of matchms.Spectrum objects, the options to configure the algorithm (whatever you want to work first on) and does a pairwise comparison on them returning some sort of numeric matrix of the similarity scores. Maybe there is already some functionality implemented in |
@michaelwitting , maybe better to discuss this in the dedicated issue #11 |
Dear @chufz @michaelwitting @hechth,
I would suggest to first collect all the code and examples you already generated and put that into the README (maybe in separate subsections?).
Would also be cool if someone could add installation requirements there.
Also, I to avoid conflicts etc, I suggest everyone works in her/his own branch (I created already some) and merge this from time to time to the main branch.
Generally, I think this will be a very cool project. I always wanted to combine our code with the really great software that is available in python. So, thanks for kick-starting it!
The text was updated successfully, but these errors were encountered: