Skip to content

Conversation

@ivyleavedtoadflax
Copy link

@ivyleavedtoadflax ivyleavedtoadflax commented Aug 10, 2019

  • Minor fixes to crf_baseline and keras code to facilitate running on py3.7
  • Add requirements.txt
  • Update README.md

@ivyleavedtoadflax ivyleavedtoadflax changed the title Fixes to allow running keras models on python 3.7 Fixes to allow running keras and crf models on python 3.7 Aug 13, 2019
@ivyleavedtoadflax ivyleavedtoadflax changed the title Fixes to allow running keras and crf models on python 3.7 Fixes to allow running keras and crf_baseline models on python 3.7 Aug 13, 2019
Squashes: DeprecationWarning: sklearn.externals.joblib is deprecated in 0.21 and will be removed in 0.23.
@ivyleavedtoadflax
Copy link
Author

@Giovanni1085 this should be good to merge if you are happy with it. Currently fixes the crf_baseline and the keras models. I've not yet tried on tensorflow, but since I've added a specific requirements.txt for each, it shouldn't be a problem.

@Giovanni1085
Copy link
Contributor

@ivyleavedtoadflax thank you for your efforts. I had in mind an update to the readme files, not edits to the code directly as this breaks alignment with what we did in the paper, plus the TensorFlow part would remain not updated.

I propose you put the 3.7 code in a separate folder, clearly marked as such, so that people can use it as they see fit. If you agree, please update the pull request accordingly. Alternatively, please feel free to fork and have the 3.7 version under your name. Thank you!

@ivyleavedtoadflax
Copy link
Author

How about we commit to a branch name python3.7? That way the code is in your repo, but doesn't interfere with the code as it was in the paper.

@Giovanni1085
Copy link
Contributor

That would be fine for me, but I worry no-one will notice its presence. You could still edit the readme file in the master branch to mention it. Alternatively, why not putting everything in a separate folder on master?

@ivyleavedtoadflax
Copy link
Author

Ok sure, I'll create a new folder when i get a moment 👍

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.

2 participants