search

MannLabs / alphapeptdeep

Deep learning framework for proteomics
Apache License 2.0
109 stars 21 forks source link

added documentation to building_block #21

Closed ammarcsj closed 2 years ago

ammarcsj commented 2 years ago

Hi Feng, this was a very interesting exercise for me. I re-ordered, renamed some classes and added descriptions. It makes sense to read over the descriptions, as I might have misunderstood things.

jalew188 commented 2 years ago

Thank you @ammarcsj . All renamed variables are very meaningful. Docs in the nbs are also great. I am going to use them.

There are two issues:

  1. The reversed function/class defining (calling before defining) causes two problems: a. my VSCode reports xxx is not defined warnings as functions/classes are defined after their calls. As we have discussed before, it is a good idea to do reversed defination inside a class defination. I don't know if it is a good idea to do reversed defining outside a class, so I checked other popular github repos such as numpy, pytorch and sklearn, they also used forward defination (defining before calling).
  2. There are some missing classes hence I cannot run MS2/RT/CCS models. You can run below codes to check if some classes are missing. 
    from peptdeep.pretrained_models import ModelManager
model_mgr = ModelManager()
ammarcsj commented 2 years ago

Hi Feng, thanks a lot for taking the time to review. Concerning your comments:

1) Forward/"backward?:D" definition: I have now changed the order to forward definition. Looking around, I think there is not a clear answer on how to approach this, so whatever you prefer (see also here). I could not reproduce the VScode warning, maybe you can double-check. 2) Missing classes: sorry, I forgot an #export statement in the NB, now it's hopefully complete. ModelManager() runs through now and I double checked in the files. 3) META name now changed

jalew188 commented 2 years ago

Thank you, looks good