madscatt / zazmol

Library for defining molecular objects to create simulation and analysis programs To install: python setup.py install dependencies: numpy, mocker
GNU General Public License v3.0
0 stars 2 forks source link

Add error handling for incorrect dcd file name #48

Open StevenCHowell opened 7 years ago

StevenCHowell commented 7 years ago

Currently when an incorrect pdb file name is passed to read_pdb an IOError is thrown, e.g., IOError: [Errno 2] No such file or directory: './output/run2.pd'. This provides helpful output to a user.

For dcd files, this is not the case. When passing an incorrect dcd file name to open_dcd_read the following line causes python to exit without any indication of why or that the code did not run successfully to completion. readheaderresult,nnatoms,nset,istart,nsavc,delta,namnf,reverseEndian,charmm=dcdio.read_dcdheader(filepointer)

This is not functional for a standalone library.

madscatt commented 7 years ago

Error handling is not addressed well in the library in a comprehensive manner. At best it has been added as needed by programs that access the library. There are many opinions on error handling in libraries. Many libraries that are to be portable and lightweight provide NO error checking and this is the standard practice in the field. This is probably going be our approach in that the developer is responsible for checking for potential errors in pass variables.

StevenCHowell commented 7 years ago

The easier a package is to learn and use, the more people will start using it. Completely exiting with no indication that there even is a problem presents an unnecessary barrier to a user. When I first starting using SASMOL, aggregated together, I spent days trying to figure out why my code ran fine but did nothing until I realized this was happening.

There are many feasible options for notifying a user of a problem that add minimal overhead. One possibility, with no added overhead (because file_io.py already imports the os module, is to add assertions:

assert os.path.exists(filename, "no such file {}".format(filename))

Another option is to use a try-except statement. I strongly recommend adding some notification.

madscatt commented 7 years ago

I don't disagree, that is why one documents the library functionality and attempts to encapsulate the expectations of a library with a contract. IF we had documentation when you started then you would not have had to spend days tracking down this error. The other policy decision is that packages that are built on this library need to handle the error messaging and there is no agreed upon way to do this. As such, people who write libraries for the most part have abandoned error handling for library methods. You can certainly find exceptions to my statement above and I do not want to inflame your opinion. If you note my comment above "probably going to be our approach" tied together with your "strong recommend" we'll agree to keep the idea open for discussion.

StevenCHowell commented 7 years ago

Just found out this completely crashes a Jupyter notebook. It took me a bit to realized what changed, that the dcd file existed on my laptop that did not on my desktop.