-
Notifications
You must be signed in to change notification settings - Fork 264
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
Discussing the dynamically created classes #845
Comments
Dynamic generation isn't necessary, though it could be useful in some situation. We can leave the data loading as is and just name the classes explicitly. |
Is there a particular situation you have in mind?
Good idea, it will keep things a bit tidier. I'm happy to have a go at this. |
If you wanted to study a tournament of randomly generated FSM strategies, for instance, it would be a huge pain to do if the classes were not dynamically generated. So if we remove that example from the main library, an advanced tutorial would be nice. Also, before you remove those guys I'd check that the strategy transformers don't cause the same problem |
I might be missing something but could this not be taken care of using inheritance from a parent FSM class? (like for the simpler memory one strategies for example)
Yeah, I was thinking that the PR should change travis to run |
Yeah maybe there is an easier way if the base strategies are parameterized, so maybe my example isn't very good. |
The dynamically created classes are creating very minor issues with type checking (and also causing slight issues with hashing in the fingerprinting repo but that's not a big deal). Is it worth perhaps revisiting whether or not they are necessary?
Perhaps we could include the data in the corresponding modules themselves (as a class attribute for each given class). This has the negative side effect of a bit more manual work to update the classes but it simplifies things (like with type checking and also removes the need for the
read_data
module).Any thoughts?
The text was updated successfully, but these errors were encountered: