ABI-CTT-Group / metadata-manager

http://sitesdev.bioeng.auckland.ac.nz/clin864/metadata-manager/build/index.html
Apache License 2.0
2 stars 0 forks source link

Repository initial setup + basic functions + examples + docs #1

Closed chinchien-lin closed 2 years ago

chinchien-lin commented 2 years ago
PrasadBabarendaGamage commented 2 years ago

Hi @LIN810116 Is there an example that can be looked at to help with reviewing?

PrasadBabarendaGamage commented 2 years ago

@LIN810116 Also possible to add the API docs to the sphinx docs folder just in the current state. Would be great if you could please also create a readthedocs from your local repo too to help with reviewing. Lets check with @nickerso to see if we can make the repo public.

p.s. should we name the repo, metadata-manager? @LIN810116 @nickerso @dbrnz what do you think?

chinchien-lin commented 2 years ago

Hi @PrasadBabarendaGamage ,

Yup renaming it to metadata-manager sounds good.

I will add some example scripts soon.

will also create a simple Sphinx doc with the API section. Should I host it on my ABI sitesdev folder?

PrasadBabarendaGamage commented 2 years ago

@LIN810116 yes, that is a good idea

It would also be great if we can create a roadmap for developing this module, indicating milestones and put some dates next to them? We could use the github project manger so that everything is in one place. Here is an initial go: https://github.com/orgs/physiome-workflows/projects?type=beta

We can discuss the roadmap at the meeting with Andre on Thurs? Would it be possible to put an example together by then for one of the workflow step?

chinchien-lin commented 2 years ago

Hi @PrasadBabarendaGamage ,

I just pushed a few examples for some basic functions, e. g. load_template(), load_dataset(), list_fields(), etc., in the examples/ folder.

I will work on applying the module in our current breast workflow (for only one workflow step for now). Should be able to have something by Thursday.

Other functions such as extract_metadta_from_dicom() and metadata_mapping() might also need to be added in either this module or the workflow module.

PrasadBabarendaGamage commented 2 years ago

Thanks @LIN810116 We can see whether we should add some convenience functions e.g. extracting metadata from dicoms files. I am just a little worried about this as different users might want to store different field and then we would have to maintain this, whereas if we provide the fundamental api e.g. metadata.set_field(...) then it would be applicable to all types of files (just up to the users to load the info they want). Shall we chat about the strategy with @dbrnz and @nickerso once we have the examples? (and develop a roadmap with some milestones : )

chinchien-lin commented 2 years ago

Hi @PrasadBabarendaGamage ,

Sorry, I seemed to miss your previous comment (we might have already discussed it in our last meeting).

I just now pushed some new commits for the metadata extraction function and the example scripts, including the workflow example.

I also added the example and API sections to the Sphinx documentation. The documentation is now hosted on the ABI server - link.

chinchien-lin commented 2 years ago

p.s. should we name the repo, metadata-manager? @LIN810116 @nickerso @dbrnz what do you think?

Hi @PrasadBabarendaGamage , @dbrnz and @nickerso,

The repo has now been renamed from physiome-metadata to metadata-manager with some related changes in the commit 2766cef.

dbrnz commented 2 years ago

Hmm, it has now become matadata-manager! A typo somewhere??

chinchien-lin commented 2 years ago

Hi @dbrnz, Oh sorry, just fixed the typo. Thank you.

chinchien-lin commented 2 years ago

Hi @PrasadBabarendaGamage @nickerso @dbrnz

I was just wondering if you are OK if I merge this PR since it is getting a bit large. This PR is basically just for the initial repository setup + some basic functions + examples. For future updates, each PR should ideally contain only one purpose.

PrasadBabarendaGamage commented 2 years ago

Looks good to me - good job Chinchien!! the examples are running - we should consider tests as part of a different issue/pull request. There is an automated test setup in the template that we could use.

chinchien-lin commented 2 years ago

good job Chinchien!! the examples are running - we should consider tests as part of a different issue/pull request. There is an automated test setup in the template that we could use.

Yes @PrasadBabarendaGamage , I will add some tests in a separate pull request