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

Simple api #1

Closed
wants to merge 10 commits into from
Closed

Simple api #1

wants to merge 10 commits into from

Conversation

hainm
Copy link
Collaborator

@hainm hainm commented Nov 23, 2015

This is not a real PR, just want to show my proposal here.

All I want is to create a simple Trajectory class so user (developer) can easily plug to.

  • Example from pytraj
import pytraj as pt
import nglview as nv

traj = pt.fetch_pdb('1l2y')
traj[:1].save('test.pdb')

structure = nv.load_file('test.pdb')

traj = nv.Trajectory(xyz=traj.xyz, topology=structure)

v = nv.TrajectoryViewer(traj)
v
  • Example from mdtraj
import mdtraj as md
import nglview as nv

traj = md.load('file.nc', 'file.gro')
traj[0].save_pdb('test.pdb')

structure = nv.load_file('test.pdb')

traj = nv.Trajectory(xyz=traj.xyz, topology=structure)

v = nv.TrajectoryViewer(traj)
v

nglview.Trajectory just need to have xyz attribute with its topology (currently I took Structure in nglview).

For structure (or Topology), I think we can use the approach from chemview (Topology is a dict, which is easy to json-ize)

def to_chemview(traj):
    import pytraj as pt
    top = {}
    top['atom_types'] = [a.element[1] for a in traj.topology.atoms]
    top['atom_names'] = [a.name for a in traj.topology.atoms]
    top['bonds'] = traj.topology.bond_indices
    top['residue_types'] = [r.name for r in traj.topology.residues]
    top['residue_indices'] = [
        list(range(r.first_atom_index, r.last_atom_index))
        for r in traj.topology.residues
    ]

    return top

- rename NGLWidget to TrajectoryViwer
    Widget is the name for developer but not for general user

- simplify NGLWidget code, still do not get the color work (got grey)
At least this one works for me:

import pytraj as pt
import nglview as nv

traj = pt.fetch_pdb('1l2y')
traj[:1].save('test.pdb')

structure = nv.load_file('test.pdb')

traj = nv.Trajectory(xyz=traj.xyz, topology={})

v = nv.TrajectoryViewer(structure=structure, trajectory=traj)
v
import pytraj as pt
import nglview as nv

traj = pt.fetch_pdb('1l2y')
traj[:1].save('test.pdb')

structure = nv.load_file('test.pdb')

traj = nv.Trajectory(xyz=traj.xyz, topology=structure)

v = nv.TrajectoryViewer(traj)
v
@arose
Copy link
Collaborator

arose commented Nov 23, 2015

So you want a simple trajectory class that just needs an array of xyz coordinates, right? What about:

import nglview
class NumpyTrajectory(nglview.Trajectory):
  def __init__(self, xyz):
    self.xyz = xyz
  def get_coordinates_list(self, index):
    return self.xyz[index].flatten().tolist()
  def get_frame_count(self):
    return len(self.xyz)

I find having a class with a method to get the coordinates of a specific frame more powerful than just an xyz property as it is easier to put logic for e.g. on-demand loading into such a method, and it is easier to extend.

Also, I was thinking of an "auto_load" method that looks at its arguments and chooses the correct interface class.

def auto_load_traj(traj):
  if hasattr(traj, "xyz"):
    return NumpyTrajectory(traj.xyz)
  elif some_check(traj):
    return SomeTraj(traj)
  raise "no fitting traj format found"

A dedicated topology format for sending to the browser is a good idea. I like the format of chemview, especially that the arrays do not mix types (i.e. not a list of atom dictionaries), which will make it easier to send them as binary data (when allowed in IPython). I will add support for that in NGL. However, I would prefer to stick to having an interface/adapter class.

Are you suggesting to rename NGLWidget to TrajectoryViewer? I would like to have a name that works for both, only structures and structures with trajectory. Or have separate widgets?

@hainm
Copy link
Collaborator Author

hainm commented Nov 23, 2015

I find having a class with a method to get the coordinates of a specific frame more powerful than just an xyz property as it is easier to put logic for e.g. on-demand loading into such a method, and it is easier to extend.

I am not sure how it is easier the the simple class Trajectory I proposed. The point here is we don't try to create many classes, it's hard to remember and confuse users. For the general users, they don't care about object oriented.

Actually, I propose nv.Trajectory is similiar to mdtraj and pytraj. You can still introduce method to it, right?
https://github.com/hainm/nglview/blob/simple_api/nglview/__init__.py#L87-L90

traj = nv.Trajectory(xyz=, top=)
coords = traj.get_coordinates(index)

With above constructor, we can add numpy 3d-array, we can add mdtraj, pytraj's data. We don't need to introduce MDTrajTrajectory, PytrajTrajectory, ... classes.

Per 'auto_load': Yes, I think it's good to make

import nglview as nv
traj = nv.auto_load_traj(external_traj)
viewer = nv.TrajectoryViwer(traj)
viwer

Are you suggesting to rename NGLWidget to TrajectoryViewer? I would like to have a name that works for both, only structures and structures with trajectory. Or have separate widgets?

Yes, I am suggesting to change to TrajectoryViwer (I "stole" this word from chemview). I don't think we need two classes for Structure and Trajectory. I am considering Structure is a Trajectory with a single snapshot, right? Trajectory have coordinates (xyz) and topology (having atom, res, ... info).

For example: given that you can make the Topology work like chemview, you can change the javascript code a bit:

  • fetch pdb from rcsb
import nglview as nv
# return a Trajectory rather a Structure
traj = nv.fetch_pdb(pdbid)
v = nv.TrajectoryViwer(traj, representations=..., **kwd)
  • load pdb from local
import nglview as nv
# return a Trajectory rather a Structure
traj = nv.load_file(pdb_filename)
v = nv.TrajectoryViwer(traj, representations=..., **kwd)
  • use scipy.io.netcdf_file to load AMBER' netcdf file
In [13]: from scipy.io import netcdf_file

In [14]: fh = netcdf_file('tz2.nc')

In [15]: xyz = fh.variables['coordinates'].data

In [16]: xyz.shape
Out[16]: (101, 223, 3)

then we can easily make nglview.Trajectory.

So you (users) don't really need to remember much. Just creating a Trajectory (even with single frame), throw the trajectory to TrajectoryViwer, and view.

@arose
Copy link
Collaborator

arose commented Nov 25, 2015

I agree, the user should not have to remember any classes, just supply an object to the widget and get a view. All the sub classes of Structure and Trajectory are for developers who want to do something special. The user-facing API should be simpler, I'll see what I can do.

@hainm
Copy link
Collaborator Author

hainm commented Nov 28, 2015

@arose; sure, wait for your decision/progress. Mean while I keep adding code to follow my idea. We can make another PR if the code is simple for users to use.

@hainm hainm closed this Nov 28, 2015
@hainm
Copy link
Collaborator Author

hainm commented Dec 11, 2015

@arose

this is an example why we should simplify the API rather creating many classes

https://gist.github.com/kennethreitz/973705

(urllib2 vs requests)

hainm pushed a commit that referenced this pull request Feb 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants