Closed cgravill closed 3 years ago
Sure, and happy with the duplication.
Looks good to me Colin, I like the contextmanager approach. Sorry I didn't take in your original post when I looked at this during the meeting - I was thinking that surely the benchmarking code was going to load ksc at some point anyway in order to benchmark it, so why not; I hope I haven't derailed your careful dependency management.
Looks good to me Colin, I like the contextmanager approach. Sorry I didn't take in your original post when I looked at this during the meeting - I was thinking that surely the benchmarking code was going to load ksc at some point anyway in order to benchmark it, so why not; I hope I haven't derailed your careful dependency management.
Thanks for the comments! It's better now, we only needed TS2K before but practically that's likely to merge with KSC so let's get the benefits of reduced duplication.
To match #829
I've used a utility method as done in TS2K as I'd rather not change sys.path for longer than needed.
I also moved that utility method up to utils.py and initially planned to use it.
However, taking a dependency from the benchmarking tool into KSC seems like it could complicate matters with it's further dependencies. Any thoughts? I'm fine with this narrow duplication if others are.I went ahead and used the one in utils.AB#19084