Skip to content

Conversation

@evanbiederstedt
Copy link

Compatibility with both 2.7 and 3.x.
Encoding/decoding strings in order to bypass this issue: h5py/h5py#289

Revisions to travis config, README, style changes

@evanbiederstedt
Copy link
Author

RE: README

As I mention, I rather prefer the motivation/details for the first few sentences in the README. I re-implemented some of the previous old README sentences, as it gives an explanation of Classic HotNet and HotNet2.

Is it clean to users given the README what num_cores=-1 means? We should probably make this more clear.

RE: C/Fortran

In principle, users should compile these extensions when installing the python package, similar to WExT. I haven't dealt with this. I don't think it is very important for the use case. I've kept setup_c.py and setup_fortran.py separate.

RE: py23 compatibility changes

--- Any .iteritems() became .items()

--- We were previously using implicit relative imports. Python does not like this----these need to be explicit relative imports

--- https://github.com/raphael-group/hotnet2/blob/master/scripts/permuteNetwork.py#L70

cores = mp.cpu_count() if args.cores == -1 else min(args.cores, mp.cpu_count)

I actually think this was a bug. Even in Python2.7, this should be mp.cpu_count(), not mp.cpu_count

I should note that variations of this line of code are used at every instance of multiprocessing, but we don't explicitly check for min(args.cores, mp.cpu_count) each time. I'm torn whether this check is necessary. I've therefore left these lines alone.

--- Tuple parameters to functions is not allowed in Python3.x. I unpack the tuples within the function, which is both 2.7 and 3.x compatible.

e.g. https://github.com/evanbiederstedt/hotnet2/blob/master/hotnet2/permutations.py#L13-L14

--- The largest change is the work around for load_hdf5() and save_hdf5()::

https://github.com/evanbiederstedt/hotnet2/blob/master/hotnet2/hnio.py#L391-L455

I save the HDF5 as type S, and decode to be type np.unicode_ upon loading the HDF5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant