search

wilzbach / msa

Modular BioJS compoment for a multiple sequence alignment
http://msa.biojs.net
Boost Software License 1.0
168 stars 79 forks source link

tree not working (Error: Cannot find module 'biojs-io-newick') #122

Closed sillitoe closed 2 years ago

sillitoe commented 8 years ago

Running snippets/tree reports the following exception.

Error: Cannot find module 'biojs-io-newick'

I seem to be able to work around this error by including msa-tnt and biojs-io-newick as regular modules (browserify) rather than the current async msa.g.package.loadPackages. Suggests the problem is down to the loading, not the modules themselves, but I haven't got much further than that.

Note: the correct files do seem to be included:

GET /node_modules/msa-tnt/build/bundle.js - 304 Not Modified - 2.3 MB
GET /node_modules/biojs-io-newick/build/biojs-io-newick.min.js - 304 Not Modified - 6.7 KB

However, it looks like msa-tnt is the full bundle that contains everything (i.e. it includes its own version of msa). At best, it looks like a redundancy that could be made slightly more efficient. At worst, this looks like it could make the universe collapse in a mess of meta-recursion.

Is it better to leave msa-tree as the container/adaptor for the components msa and tnt?

wilzbach commented 8 years ago

However, it looks like msa-tnt is the full bundle that contains everything (i.e. it includes its own version of msa).

Yep the adapter is horrible.

At worst, this looks like it could make the universe collapse in a mess of meta-recursion.

I agree.

Is it better to leave msa-tree as the container/adaptor for the components msa and tnt?

msa-tnt has historic reasons. David and I tried to find a nice way to combine biojs components which failed because components need to have some common API.

Imho the (best) way to move forward is

1) to have a small loader that uses the cdn js builds, but can be customized. the logic should be as simple as currently: try to load async if variables is not there), but allow the user to specify his/her own paths and detect global variables

loadPackage("tnt") -> cdn.bio.sh/tnt/... || ./tnt.js

2) The tree adapter should be included directly as msa plugin - it can then bootstrap the tree and set events. AFAIK https://github.com/daviddao/exelixis already includes the newick parser, but tnt.tree is the original component and is still maintained and updated. Currently the main problem is that the msa-tnt component has the assumption that it has to construct both components.

sillitoe commented 8 years ago

msa-tnt has historic reasons. David and I tried to find a nice way to combine biojs components which failed because components need to have some common API.

Thanks - I figured that might be the case. I'm currently working on an app that combines components - I figured I would use this as a template, but sounds like there might be better examples?

David and I tried to find a nice way to combine biojs components which failed because components need to have some common API.

Is it worth choosing a particular MVC framework (e.g. Backbone) and getting people to inherit from a plugin view?

Anyway, ignoring for a moment whether this should be using msa-tnt, the original issue was that the dynamic loading of packages in msa didn't appear to be working ("Error: Cannot find module 'biojs-io-newick'"). Seems that dynamic loading is going to be useful in lots of places - I'm happy to invest time understanding/fixing if this still seems the best way of doing things.

wilzbach commented 8 years ago

Is it worth choosing a particular MVC framework (e.g. Backbone) and getting people to inherit from a plugin view?

Yes absolutely. Backbone is probably the easiest, however it has its limitations. I think currently in the msa a) it's quite tricky to see where events come from and b) to extend/overwrite existing views. That being said - I still want to modularize the msa and imho the best way to go forward is to split it up into multiple web components. You should be able to observe this progress over the next weeks.

Yes dynamic loading is important as we want to ship a slim package that can be extended. I will reply to the color issue later. Sorry that this got lost.

On May 10, 2016 6:06:30 PM GMT+03:00, Ian Sillitoe notifications@github.com wrote:

msa-tnt has historic reasons. David and I tried to find a nice way to combine biojs components which failed because components need to have some common API.

Thanks - I figured that might be the case. I'm currently working on an app that combines components - I figured I would use this as a template, but sounds like there might be better examples?

David and I tried to find a nice way to combine biojs components which failed because components need to have some common API.

Is it worth choosing a particular MVC framework (e.g. Backbone) and getting people to inherit from a plugin view?

Anyway, ignoring for a moment whether this should be using msa-tnt, the original issue was that the dynamic loading of packages in msa didn't appear to be working ("Error: Cannot find module 'biojs-io-newick'"). Seems that dynamic loading is going to be useful in lots of places - I'm happy to invest time understanding/fixing if this still seems the best way of doing things.

You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/wilzbach/msa/issues/122#issuecomment-218187105

sillitoe commented 8 years ago

No problem.

I was going to prepare a separate PR without any mention of d3 - happy to postpone that until there's a good answer for the dynamic loading.

On 10 May 2016 at 16:58, Sebastian Wilzbach notifications@github.com wrote:

Is it worth choosing a particular MVC framework (e.g. Backbone) and getting people to inherit from a plugin view?

Yes absolutely. Backbone is probably the easiest, however it has its limitations. I think currently in the msa a) it's quite tricky to see where events come from and b) to extend/overwrite existing views. That being said - I still want to modularize the msa and imho the best way to go forward is to split it up into multiple web components. You should be able to observe this progress over the next weeks.

Yes dynamic loading is important as we want to ship a slim package that can be extended. I will reply to the color issue later. Sorry that this got lost.

On May 10, 2016 6:06:30 PM GMT+03:00, Ian Sillitoe < notifications@github.com> wrote:

msa-tnt has historic reasons. David and I tried to find a nice way to combine biojs components which failed because components need to have some common API.

Thanks - I figured that might be the case. I'm currently working on an app that combines components - I figured I would use this as a template, but sounds like there might be better examples?

David and I tried to find a nice way to combine biojs components which failed because components need to have some common API.

Is it worth choosing a particular MVC framework (e.g. Backbone) and getting people to inherit from a plugin view?

Anyway, ignoring for a moment whether this should be using msa-tnt, the original issue was that the dynamic loading of packages in msa didn't appear to be working ("Error: Cannot find module 'biojs-io-newick'"). Seems that dynamic loading is going to be useful in lots of places - I'm happy to invest time understanding/fixing if this still seems the best way of doing things.

You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/wilzbach/msa/issues/122#issuecomment-218187105

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub https://github.com/wilzbach/msa/issues/122#issuecomment-218204511

Ian Sillitoe | Senior Research Fellow CATH Manager | www.cathdb.info University College London | 627 Darwin, Gower Street, WC1E 6BT

wilzbach commented 8 years ago

related to #150

wilzbach commented 8 years ago

Should be fixed with #150 - a proper solution however would be only to load tnt and put the msa-tnt adapter code into the msa viewer.