search

MobleyLab / Lomap

Alchemical mutation scoring map
MIT License
37 stars 17 forks source link

change the code and test code to be compatible with networkx 2.0 #34

Closed shuail closed 5 years ago

shuail commented 6 years ago

change the code to be compatible with networkx version > 2.0 but will not longer support the version lower than 2.0

davidlmobley commented 6 years ago

Ah, currently tests are failing because networkx 2.1 (which this is pinned to) isn't compatible with py34 for some reason (or at least that's what the tests on travis say). Maybe test only on py27, py35, py36? (Remove py34 test, add py35 and py36?). Thoughts, @shuail ?

shuail commented 6 years ago

@davidlmobley I tried to change the python version in .travis.yml (not quit sure if I did the correct thing...), the testing breaks in an odd manner, will come back to this soon.

davidlmobley commented 6 years ago

Hmm, still not passing @shuail .

shuail commented 6 years ago

@davidlmobley yes, seems rdkit get stuck with python 3X, it's a little puzzle to me since I didn't touch the rdkit part and from the log of previous testing, rdkit seems to be happy with python3X... I have a pretty tight workload this week, will come back to this maybe the end of this week. Sorry about the delaying...

shuail commented 6 years ago

@davidlmobley sorry, I have a very busy week/weekend and didn't have a chance to look at this yet. Probably will have sometime after the free energy workshop.

davidlmobley commented 6 years ago

@shuail - I just noticed we still have this open. Were you going to get a chance to look?

shuail commented 6 years ago

@davidlmobley sorry, I totally forget this... Will take a deep look this weekend.

shuail commented 6 years ago

@davidlmobley I tried to pin rdkit to the latest version in meta.yaml file while it seems that travis doesn't recognize the version label so I roll it back to the default rdkit version. The default rdkit version is rdkit-2016.03.1 which is specified in the nividic channel. It seems that this version of rdkit is not compatible with python 3.5 and 3.6, so if possible maybe we should update the default rdkit in nividic channel to a more advanced version.

davidlmobley commented 5 years ago

@ppxasjsm - https://xkcd.com/1296/

Thanks for all your work on this. :)

ppxasjsm commented 5 years ago

I would propose the following with regard to this pull request.

Do not merge it into master. Master should only contain well tested stable code. The current version is mostly ok, but there are various issues that do need fixing. For this reason, I have created a devel branch, which I will use to add smaller feature requests rather than having at some point one enormous pull request.

The devel branch contains all the changes of the networkx2 branch but has a working travis build now. There are some dependency issues I'll have to look into properly, but I'd like to do that once having a test suit that actually passes and doesn't skip tests in order to be able to work with networkx2.

My proposed way of approaching this:

Therefore, I'd reject the pull request (it is fully incorporated in devel, and none of the changes are lost). Make changes with smaller features to devel leading to a fully stable version of the code which can then be used as a release. Once the release actually passes all tests, create new conda recipe and push onto pypi. The networkx2 branch can in theory then be deleted now.

davidlmobley commented 5 years ago

This sounds reasonable to me, @ppxasjsm . Shall I close this one now, then?

davidlmobley commented 5 years ago

And congrats on getting it to pass!

ppxasjsm commented 5 years ago

Yes I think it would be safe to close it now. Just a little note to maybe my future self. Installing pyqt version 4 via conda removes the packaged rdkit 2018 and matplotlib version >2.