search

LArbys / LArCV

Liquid Argon Computer Vision
11 stars 9 forks source link

apicaffe_cleanup into develop #55

Closed drinkingkazu closed 7 years ago

drinkingkazu commented 7 years ago

apicaffe_cleanup branch is ready to be merged into develop, I believe.

drinkingkazu commented 7 years ago

... and of course I am happy to resolve conflict

drinkingkazu commented 7 years ago

Conflict is resolved. @vgenty @erezcoh4 @twongjirad @ahack379 @coreyjadams I would like to merge this branch back into develop. Please give a go, and would be great if you can even test it!

erezcoh4 commented 7 years ago

sounds great! would gladly help test it

drinkingkazu commented 7 years ago

For testing apicaffe_cleanup branch, if anyone can try what "you used to do w/ develop branch" with this release candidates. If works, test pass :)

In future it should use a set of unit test, a development of unit test may be coordinated with this effort https://github.com/LArbys/LArCV/issues/51 but by no means we should delay the release.

A release candidate for develop branch before this merge is also made: https://github.com/LArbys/LArCV/tree/release/v01.00.00

twongjirad commented 7 years ago

Crap. I fear I mess this up with my PR.

twongjirad commented 7 years ago

Looked into the conflict. Am I right, supera.fcl is symlinked to

/Users/kazuhiro/PlayGit/LArCV/app/Supera/larfmwk_shared/supera.fcl ?

This can't be right. Is the intention to bring this file into the repository?

drinkingkazu commented 7 years ago

Brought up to date w/ develop.

@twongjirad the symbolic link was intentional to share an "example" configuration file among larsoft and larlite interface so that we avoid a mistake of "oh I updated here but forgot there" kind of things. not important so I just pushed a new replacement (physical file instead of symbolic link).

erezcoh4 commented 7 years ago

Kazu, could you please make the following modification? I am having problems w/ CV2layout that might occur to other users as well, and the only way to load view_rgb is by relaxing its requirement

in mac/pyrgb/display/rgbdisplay.py, in line 259, change from self.cv2_layout = CV2Layout() to try: self.cv2_layout = CV2Layout() except: print "no CV2" self.cv2_layout = None pass

drinkingkazu commented 7 years ago

@erezcoh4 Is this something already fixed in develop? If so, this will be in place once merged!

drinkingkazu commented 7 years ago

@erezcoh4 I see similar lines in develop here: https://github.com/LArbys/LArCV/blob/develop/mac/pyrgb/display/rgbdisplay.py#L20-L23

If that works for you then we will have it once merged :)

drinkingkazu commented 7 years ago

@ahack379 confirmed her module (tested under PlayGround) works fine