g2nb / igv-jupyter

Extension for Jupyter which integrates igv.js
MIT License
154 stars 14 forks source link

Changing References does nothing #11

Closed ijoseph closed 5 years ago

ijoseph commented 6 years ago

Greetings, thanks for a great piece of software.

When I attempt to change the reference used (e.g., in order to use hg38 rather than hg19) according to the syntax specified in the example: igv.Reference(fasta_url=...), nothing happens. I must use camelCase for my parameter names instead: igv.Reference(fastaURL=...).

I feel that is a bug because at the very least, there should be some sort of warning when my attempt to update the reference failed. Instead, it silently and dutifully serves me hg19 coordinates.

I think this is due to some issue with the to_camel_case method potentially not working.

a-slide commented 5 years ago

ref = Reference(id="GRCm38", fasta_url="./References/GRCm38_PRI_genome.fa.fai", index_url="./References/GRCm38_PRI_genome.fa.fai")

This worked for me, but it required a lot of tinkering around...

Such a shame the package has not been documented and developed further, because the idea is awesome

jrobinso commented 5 years ago

Thanks @ijoseph . The package hasn't been documented and developed because it didn't really catch on, and I'm a single person spread across several projects. Of course, maybe it didn't catch on because its not documented well. What features or additions beyond proper documentation do you think it needs to be a viable package? I do not personally use Jupyter.

a-slide commented 5 years ago

Hi @jrobinso, First, thanks a lot because I love the concept of having IGV running inside Jupyter. It is very handy to have a nice self-contained end to end analysis.

I would say that one of the main drawback is that the interface is not very pythonic, which makes everything hard to use in jupyter. Since the function/class args are not explicitly declared in the signatures, the only way to find them is to read the raw code. I understood that it gives more flexibility, but declaring the args + having a docstring for each functions would actually make the package usable without having to guess the arguments values. I have a few other suggestions but I suppose it would be best to open an issue for each !

jrobinso commented 5 years ago

@a-slide Good point! It's not pythonic because I'm not pythonic, however, I am using python now for another project and I am getting better. I would like to come back to this one and wrap it up well, so please do open issues for your other suggestions. It would be helpful.

ijoseph commented 5 years ago

Hm, yes, agreed with, @a-slide . It's almost there, just more documentation and error checking* would make a huge positive difference, I think.

But, this is my last day in the BioTech field (leaving my current company for a non-BioTech one), so I suppose I have little horses in the race moving forward…

*This is tangentially-related to being Pythonic; basically, just conforming to Pythonic expectations about there being some documentation, and duck-typing working by throwing errors if something unexpected is passed (rather than silently failing).

jrobinso commented 5 years ago

@a-slide I've started a major revision of this package. The new interface is much simpler, particularly if you are familiar with igv.js. Basically it mimics the igv.js API using python dictionaries where json-like config objects are specified in igv.js. The dictionaries are converted to JSON and passed to the front end as messages. I also added docstrings, but I'm not sure what you mean by "declaring the args". Python is not my first or even second language, but have a look and feedback is welcome.

I have not pushed this to pypi yet.