Skip to content

Conversation

@tomwesolowski
Copy link
Contributor

No description provided.

@Zulko
Copy link
Owner

Zulko commented Jun 13, 2015

Sorry for the delay, I'm completely under the water right now. Thanks very much for all this, it's very interesting and I'll soon merge it. At this point I have several requests, by order of importance:

  • You should update the documentation so that your contributions become accessible to the users. Tell me if you don't have time or don't know how to use sphinx, I'll find the time to do it. Once it is done I can make a new release of easyAI on PyPI. While we are at it, it would be great if you appeared in the README. (something like "EasyAI is an open source software originally written by Zulko and released under the MIT licence. MTD and MTFS algorithms were contributed by Mrfesol.")
  • Ensure that the code runs in Python 3. That mostly means writing print() instead of just print, and there may be some issues with lambdas (unsure).
  • As the package is also educational, can you try to add a few comments to explain why you do things when it's not trivial. For instance self.hash(key) & len(self.dict)-1 is a little cryptic, and i have no idea why self.modulo = 1023 in HashTT
  • Please enforce PEP8 (some lines are too long)
  • The examples have become a little strange, you define several variables a1, a2, a3 but only use a1 and a3 for instance. Please keep the examples as simple as possible for a newcomer.

Cheers,

@tomwesolowski
Copy link
Contributor Author

Hello,

I really don't know how to use sphinx and I don't want to break anything, so I think it would be the best if you could do it. I made small changes to the project, check it out.

@JohnAD
Copy link
Collaborator

JohnAD commented May 9, 2017

@mrfesol , I would be very interested in seeing your changes added. I'd be happy to modify the documentation for you. May I pull your branch and make the documentation changes?

@tomwesolowski
Copy link
Contributor Author

tomwesolowski commented May 23, 2017

@JohnAD Sure.

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.

3 participants