Child pages
  • Code Review 04-24-14
Skip to end of metadata
Go to start of metadata

Code Review Meeting for the Reciprocal Space Mapping project was held on 4/24/14. This page contains list of participants, key event timeline, and summary of issues that were discussed at the meeting. Unless stated otherwise, all reviewers agreed on issues listed below.

Note that the unedited compilation of notes from all reviewers contains more detailed description of various problems, as well as additional comments and suggestions for improvements.

Participants

Author: John Hammonds
Reviewers: Arthur Glowacki (Scribe), Tim Mooney, Sinisa Veseli (Moderator)
Observers: Nicholas Schwarz

Key Events

  • 2014-03-25: Sinisa sent out Review Meeting Notice.
  • 2014-04-17: John tagged the code and sent out additional instructions for running it (tag URL: https://subversion.xray.aps.anl.gov/RSM/rsMap3D/tags/2014-04_CODE_REVIEW/).
  • 2014-04-23: Each reviewer prepared their own list of issues for the Review Meeting.
  • 2014-04-24: Review Meeting was held @ 10:00am (C2200). Arthur prepared meeting notes.
  • 2014-04-25: Sinisa prepared review report outlining issues that may have to be addressed.
  • 2014-04-29: Followup Meeting John Hammonds and Nicholas Schwarz.

Issues 

Bugs/Defects
  1. File Tab: Cancel button calls uninitialized variable if pressed before loading.
  2. File Tab: Entering wrong number of arguments into "Detector ROI" or "Number of Pixels To Average" results with a generic "list index out of range" exception, rather than with a specific error message.
  3. File Tab: Supplying wrong xml file for Detector config results with a "NoneType object has no attribute findall" error, rather than with a specific error message.
  4. File Tab: User's choice for input project file is not limited via filters.
  5. File Tab: GUI components are not disabled while loading configuration/data files.
  6. File Tab: Loading incorrect xml file does not display error / warning message.
  7. File Tab: "Cancel" button is always enabled, even if there is nothing that can be canceled.
  8. File Tab: Loading spec files copied to a different location without underlying images does not indicate any errors, even though application will not work.
  9. File Tab: There is no application feedback ("working...", "done") on the state of progress through the load operation.
  10. File Tab: GUI does not permit both Bad Pixel File and Flat Field Correction, even though underlying code allows it.
  11. Process Data Tab: User is not notified about where processing output is being stored.
  12. Data Range Tab: Supplying min argument that is greater than max argument does not display error message.
  13. Data Range Tab: Reset button is always enabled, and does not seem to be doing anything other than converting integer to float.
  14. Scans Tab: Column values for theta, chi, etc, can be changed to strings.
  15. Scans Tab:  Numbers ("X min", etc.) appear editable, but they are not.

  16. Process Data Tab: Processing cannot be cancelled.
  17. Image Window: It is not connected to the main window. If closed, it cannot be brought back.
  18. Image Window: Mouse action is unintuitive, with no obvious way to recover the initial or default orientation.

  19. Sector33SpecDataSource.py: If orientation is not found in the input spec file, corresponding variable is not set to None.

  20. gridmapper.py and polemapper.py: Those two files appear to be the same.

Documentation/Installation/Usability/Examples
  1. User Guide: It is written in MS Word.
  2. Confluence Examples Page/User Guide: Requirement for specific data directory layout with respect to spec file is not documented (i.e., copying spec files into different location does not work).
  3. Confluence Examples Page/User Guide: Documentation does not clearly outline what is needed to run the software with sample spec files.
  4. Confluence Installation Page: Required python packages do not have links.
  5. Confluence Installation Page: Required python version and versions for all dependencies are not listed.
  6. Confluence Installation Page: Several python package dependencies are not listed.
  7. Confluence Installation Page: Installation notes for windows are out of date.
  8. Confluence Installation Page: Explicit software installation instructions do not exist for linux/mac.
  9. Installation (RHEL 6.x): Software cannot be installed on RHEL 6.x using standard OS python packages (due to old version of numpy).
  10. Installation (Darwin 13.x): PySide namespace may be used instead of PyQt4 namespace.
  11. Installation (Fedora 20): "Image" module is under PIL namespace.
  12. Application Runtime: Data processing typically results in very large memory consumption (> 5GB at times), and gui becomes unresponsive.
  13. Application Runtime: Application cannot be stopped using CTRL-C from the command line.
  14. Application Runtime: Several errors appear as exceptions on the terminal, without alerting user on the GUI screen that something went wrong.
  15. Application Runtime: Files named "sys?" and "traceback?" appear on the filesystem (Note: only Tim experienced this problem.)
  16. Application Runtime: Some input files (e.g. "YSZ111_1.spec") result in the "QObject::connect: Cannot queue arguments of type 'Qt::Orientation'" error.
Design/Coding Style/Best Practices/Extensibility
  1. rsmEdit.py (line 100): Generic exception caught in load always throws file error, which may mask other error types.
  2. fileform.py (line 6): Statement imports all names from PyQt4.QtCore package. Similar issues can be found elsewhere.
  3. fileform.py: Misspelled words in several comments.
  4. Several classes and class methods have empty documentation strings.
  5. Code documentation does not list author/contact information (e.g., "APS/Software Services Group").
  6. Project specific exceptions do not have common base.

  7. Common strings that repeat are not constants (e.g. 'images/%s').

     

     

     

  • No labels