[searchlight] sys.exit used in _get_block_data #156

Activity

1. mihaic

   added

   bug

   on Jan 11, 2017
2. mihaic

   assigned

   mjanderson09

   on Jan 11, 2017
3. mihaic

   added

   easy

   on Jun 14, 2017
4. mihaic

   unassigned

   mjanderson09

   on Jun 14, 2017
5. YSanchezAraujo commented on Jun 21, 2017

   

   YSanchezAraujo

   on Jun 21, 2017

   More actions

   @mihaic are you saying the exception should be raised here instead of here
6. mihaic commented on Jun 21, 2017

   

   mihaic

   on Jun 21, 2017

   MemberAuthor

   More actions

   @YSanchezAraujo, thanks for looking into this issue.

   Indeed, I am saying that an exception should be raised in distribute, such that the user code can decide what to do, instead of exiting the program as we are doing now. This is the approach we are using everywhere else in BrainIAK.

   In practice, in addition to raising an exception in _get_block_data, the exception should be caught and a new one raised in distribute (so the user does not see the implementation details in the traceback).

   Instead, it would be simpler to check the input shape in distribute and just remove the else code from _get_block_data.
7. YSanchezAraujo commented on Jun 21, 2017

   

   YSanchezAraujo

   on Jun 21, 2017

   Last edited by YSanchezAraujo

   More actions

   thanks for the response, some questions:

   is the mask argument guaranteed to be a numpy array, and or the 4d arrays inside subjects, should there be an instance check and maybe a np.array(<argument>) if isinstance(<argument>, np.ndarray) returns false?

   thinking of something like:

   assert mask.ndim == 3, "mask should be a 3D array"
   
   for (idx, subj) in enumerate(subjects):
       assert subj.ndim == 4, \
       "expected 4D data for index {} in subjects".format(idx)
8. mihaic commented on Jun 21, 2017

   

   mihaic

   on Jun 21, 2017

   MemberAuthor

   More actions

   We should also check mask. And arguments should be np.ndarray instances. It is better if users explicitly apply np.array to other data structures themselves and deal with potential issues.
9. mihaic

   closed this as completedin e2dd0a7on Jun 23, 2017
10. danielsuo

    added 2 commits that reference this issue on Nov 16, 2017

    minor: single quote to double (#156)

    c3335ed

    Use redismodules for task table and result table. (#156)

    ...

    985c424