metaspace2020 / sm-engine

Engine for metabolite annotation for imaging mass spectrometry
Apache License 2.0
7 stars 5 forks source link

Initial version of MS acquisition geometry storage #21

Closed iprotsyuk closed 6 years ago

iprotsyuk commented 7 years ago

The PR enables sm-engine to store a json object in the DB, which describes the geometry of mass spec acquisition in terms of pixel layout and pixel sizes.

Some ideas are maybe not implemented in the best way. I'll highlight them in the diff shortly.

The acquisition geometry can be stored, but nothing is implemented to retrieve it, e.g. for the web app. I'm planning to add necessary endpoints later, likely within this PR, after the current code is reviewed. I also haven't yet updated any tests that were affected by the changes.

intsco commented 7 years ago

@iprotsyuk Please make sure the code passes existing tests. It would be nice to have tests for the new classes as well. Once it's done, we'll do merge

intsco commented 7 years ago

@iprotsyuk By the way, I didn't see a parser_factory for LC data

iprotsyuk commented 7 years ago

@intsco, yes, I'll add new tests. As for the LC data parser factory, would you suggest to keep it in the master branch or in a separate "LC branch" instead?

intsco commented 7 years ago

@iprotsyuk all the code should eventually be merged into master. Otherwise, it will be quite difficult to handle cross updates of both branches.

iprotsyuk commented 7 years ago

@intsco, alright. I wasn't sure about this, so didn't add the LC implementation. Will add it to this PR as well

iprotsyuk commented 7 years ago

@intsco, I think all the discussed points are addressed in my latest commits

intsco commented 6 years ago

@iprotsyuk the next would be pulling all the changes to master and testing it components together first locally. Have you finished with changes of the graphql and web app?

iprotsyuk commented 6 years ago

@intsco, not yet. I thought these components could be merged independently of each other provided no existing tests fail and new tests added where appropriate. But if for you it's easier to merge everything at once, we can wait till other layers are ready.