Skip to content
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

jami-core #17

Closed
18 of 24 tasks
tschaka1904 opened this issue Jul 29, 2016 · 22 comments
Closed
18 of 24 tasks

jami-core #17

tschaka1904 opened this issue Jul 29, 2016 · 22 comments

Comments

@tschaka1904
Copy link
Member

tschaka1904 commented Jul 29, 2016

General workshop guidance

https://github.com/MICommunity/psi-jami/tree/master/jami-core/src/main/java/psidev/psi/mi/jami

  • analysis/graph
    • README.md
    • Test coverage
    • Java example
  • binary
    • README.md
    • Test coverage
    • Java example
  • datasource
    • README.md
    • Test coverage
    • Java example
  • exception
    • README.md
    • Test coverage
    • Java example
  • factory
    • README.md
    • Test coverage
    • Java example
  • listener
    • README.md
    • Test coverage
    • Java example
  • model
    • README.md
    • Test coverage
    • Java example
  • utils
    • README.md
    • Test coverage
    • Java example
@tschaka1904
Copy link
Member Author

tschaka1904 commented Jul 29, 2016

What are psi-jami/jami-core/src/main/resources/interactorDatabase.properties & interactorType.properties used for? We need description where and for what it is used.

DefaultInteractorFactory.java uses both properties files.

Loads some properties to recognize interactor type from different MI terms

WARNING: These MI terms are hardcoded. Maybe use web services instead? Could cache temp file etc. See #32

@tschaka1904
Copy link
Member Author

tschaka1904 commented Jul 29, 2016

Is the a need for the psi-jami/jami-core/src/main/assembly/descriptor.xml?

@tschaka1904
Copy link
Member Author

Should we use a lib for checksum instead of psi-jami/jami-core/src/main/java/psidev/psi/mi/jami/utils/checksum?

@tschaka1904
Copy link
Member Author

jami-core/datasource provides interfaces for of sources and stream to receive interaction data.

@julie-sullivan
Copy link

@tschaka1904 That /assembly/description.xml file is a mistake and should be removed. I confirmed this with @danielabutano. Maven has assembly.xml but not description.xml. Why would you need to create a custom zip file anyway?

tschaka1904 added a commit that referenced this issue Aug 9, 2016
… agreed to remove this file. ....Let's see what happens
@julie-sullivan
Copy link

Interaction and InteractionEvidence is in the model package. BinaryInteraction and BinaryInteractionEvidence is in the /binary directory. Why?

@julie-sullivan
Copy link

Also, the word binary means something in computer science. Having binary directory is confusing! This should be renamed.

@julie-sullivan
Copy link

jami-core/analysis/graph is used to calculate cliques, which is used to calculate inferred interactions. This is used when writing interaction XML.

What are inferred interactions used for?

@julie-sullivan
Copy link

`jami-core/binary', in addition to having a bunch of classes that should be in the model, also handles the expansion.

@tschaka1904
Copy link
Member Author

Should we refactor utils or do we want to keep it?

@julie-sullivan
Copy link

image

@tschaka1904
Copy link
Member Author

We should move the wrapper classes out of psidev.psi.mi.jami.binary.impl. Maybe into utils?

@julie-sullivan
Copy link

image

binary package is tested well.

@julie-sullivan
Copy link

image

Overall test coverage is okay for psi-core

@julie-sullivan
Copy link

julie-sullivan commented Aug 9, 2016

jami-core/datasource has no tests, but that's okay. It's just a bunch of interfaces. Same for jami-core/exceptions

@tschaka1904
Copy link
Member Author

About inferred interactions Sandra said:

I think this was the terminology we used for cases such as the curated complexes, or also when we were attempting to add cases where a binding site was created by the binding of 2 molecules then a third protein bound into it.

@julie-sullivan
Copy link

julie-sullivan commented Aug 9, 2016

Could not run either test in psi-core/factory

Aug 09, 2016 11:58:48 AM psidev.psi.mi.jami.factory.MIDataSourceFactory getMIDataSourceWith
WARNING: We cannot instantiate data source of type class psidev.psi.mi.jami.datasource.MockExperimentDataSource with the given options.
java.lang.IllegalArgumentException: no source option
    at psidev.psi.mi.jami.datasource.MockExperimentDataSource.initialiseContext(MockExperimentDataSource.java:20)
    at psidev.psi.mi.jami.factory.MIDataSourceFactory.instantiateNewDataSource(MIDataSourceFactory.java:156)
    at psidev.psi.mi.jami.factory.MIDataSourceFactory.getMIDataSourceWith(MIDataSourceFactory.java:55)
    at psidev.psi.mi.jami.factory.MiDataSourceFactoryTest.test_missing_file_data_source_options(MiDataSourceFactoryTest.java:35)

See #30

@tschaka1904
Copy link
Member Author

screen shot 2016-08-09 at 12 11 30

test coverage of model package

@tschaka1904
Copy link
Member Author

screen shot 2016-08-09 at 12 19 45

test coverage of factory package

@tschaka1904
Copy link
Member Author

tschaka1904 commented Aug 9, 2016

screen shot 2016-08-09 at 12 21 37

test coverage of listener package. There are no tests

@tschaka1904
Copy link
Member Author

screen shot 2016-08-09 at 12 22 58

test coverage of utils package.

@julie-sullivan
Copy link

Datasource is a bad name for this package. (Maybe because a data source is a specific thing in InterMine!).

I would prefer "stream helper" or stream handler. Something that indicates what this package actually does. Even better than documentation!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants