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.

Sign up for GitHub

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

Jump to bottom

[BUG] Usage of eval is bad practice: use factory design pattern #77

Closed
raphaelreinauer opened this issue May 6, 2022 · 2 comments · Fixed by #86
Closed

[BUG] Usage of eval is bad practice: use factory design pattern #77

raphaelreinauer opened this issue May 6, 2022 · 2 comments · Fixed by #86
Assignees
@matteocao
Labels
bug Something isn't working mid priority The issue is to be solved when possible, as this is a relevant addition to the library
Milestone
Giotto-deep release

Comments

@raphaelreinauer
Copy link
Collaborator

In the TorchDataLoader class:

giotto-deep/gdeep/data/torch_datasets.py

Line 84 in f62b2ef

string_train = 'datasets.' + self.name + '(root="data",' + \

the eval method for strings is used, which is not type-checked and makes it hard to detect errors.
Hence it should not be used anywhere in the library.
Other parts in the library also use eval; this is only one example.
@raphaelreinauer raphaelreinauer added the bug Something isn't working label May 6, 2022
@matteocao matteocao self-assigned this May 8, 2022
@matteocao matteocao added this to the Giotto-deep release milestone May 8, 2022
@matteocao matteocao added the mid priority The issue is to be solved when possible, as this is a relevant addition to the library label May 8, 2022
@matteocao
Copy link
Contributor

will change this using your refactoring design

@matteocao matteocao linked a pull request May 12, 2022 that will close this issue
Suggestion for dataset factory template to replace gdeep.data.torch_datasets.TorchDataLoader #80
Closed
3 tasks
@matteocao matteocao changed the title [BUG] Usage of eval is bad practice. [BUG] Usage of eval is bad practice: use factory design pattern May 17, 2022
@matteocao matteocao linked a pull request May 22, 2022 that will close this issue
refactoring datasets #86
Merged
9 tasks
@matteocao
Copy link
Contributor

solved by PR #86

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@matteocao matteocao

Labels
bug Something isn't working mid priority The issue is to be solved when possible, as this is a relevant addition to the library
Projects
None yet
Milestone
Giotto-deep release
2 participants
@raphaelreinauer @matteocao