Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Investigate build and cicd issues #206

Merged
merged 5 commits into from
Apr 7, 2021

Conversation

henrifroese
Copy link
Collaborator

There are problems with dependencies (spacy and gensim especially) leading to failing builds.

@jbesomi
Copy link
Owner

jbesomi commented Apr 6, 2021

I see, we should probably define the variable name for each library to avoid these kinds of issues also in the future.

- stop class HeroTypes from inheriting from pd.Series and pd.DataFrame. Inheriting from them is not necessary at all, and causes issues with newer python versions
- skip kmeans doctest as we cannot be sure if it evaluates on [1,0,1,0] or [0,1,0,1] on different builds
- for tests with binary categories (0, 1), test whether either the result or the inversion of the result matches the expected value. This tests the same functionality but prevents failures on different OSes
@henrifroese henrifroese changed the title [DO NOT MERGE] Investigate build and cicd issues Investigate build and cicd issues Apr 7, 2021
@henrifroese
Copy link
Collaborator Author

@jbesomi build issues are now fixed with this pull request, can be merged.

  • set spaCy to <3.0.0
  • set gensim>=3.6.0,<4.0
  • change class HeroTypes(pd.Series, pd.DataFrame) to class HeroTypes without base classes. The base classes are not needed and give a method resolution order error for newer python version
  • test for both possibilities of equality for categorical results: e.g. when we test kmeans and expect result [1,0,1,0] and kmeans gives us the result [0,1,0,1] (which as 1 and 0 are just categories is functionally the same), we also accept that result

@henrifroese henrifroese marked this pull request as ready for review April 7, 2021 15:19
@henrifroese henrifroese added the bug Something isn't working label Apr 7, 2021
@jbesomi jbesomi merged commit f4bb900 into jbesomi:master Apr 7, 2021
@jbesomi
Copy link
Owner

jbesomi commented Apr 7, 2021

Very good job! 🎉 These parts were tricky yet very important! Thanks a lot, Henri 👍 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants