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

new dataset changes #224

Draft
wants to merge 6 commits into
base: develop
Choose a base branch
from
Draft

Conversation

Rashmil-1999
Copy link
Contributor

This draft PR introduces a new Dataset class with new read and write methods to hide the underlying mechanism of reading and writing datasets and modularizes the Input/Output Classes. I have left TODO's where we still need to discuss validation mechanisms.

@Rashmil-1999 Rashmil-1999 linked an issue Oct 25, 2022 that may be closed by this pull request
Copy link
Contributor

@nunezco2 nunezco2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks good, but we need a couple of things:

a. Use your code to implement ExampleAnalysis in order to have a sense of how the code would be instantiated.

b. Pass the PEP8 verifier to make sure the formatting is adequate.

@Rashmil-1999
Copy link
Contributor Author

Yes, I was thinking of implementing it somewhere so we can understand what changes can be made. Just have one question.
Do I have to make changes in the BaseAnalysis?

@Rashmil-1999
Copy link
Contributor Author

For the PEP8, how to do that? I didn't understand what you meant by "Pass the PEP8 verifier". Did you mean to give PEP8 verifier object? or to pass the PEP8 formatting test successfully?

@nunezco2
Copy link
Contributor

nunezco2 commented Nov 1, 2022

Apologies for the unclear explanation. I should have said "pass the PEP8 formatting test correctly".

@Rashmil-1999
Copy link
Contributor Author

Oh, okay so is there a command I can run to format it correctly? because my pycharm doesn't auto format for some reason. I tried using the pep8 config file.

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.

Refactoring the Dataset class
2 participants