Closed PhungVanDuy closed 1 year ago
Hey @PhungVanDuy looking good! One thing that will need to be changed though is to use our interfaces for code datasets. You can see an example of how this works with the stackexchange dataset: https://github.com/CarperAI/Code-Pile/blob/main/codepile/stackexchange/stackexchange.py
Essentially, we want to follow the same style for all our datasets so that the code is easier to manage and extend
@ncoop57 I just have a quick refactor, please check.
Hello, Daica. Thanks for the PR. PRs should go to the working
branch not the main
Hello, Daica. Thanks for the PR. PRs should go to the
working
branch not themain
Thank for your comment, just changed!
@PhungVanDuy looking good! added some comments
@PhungVanDuy added some comments. I think it is almost ready to merge once the unit test issues are resolved!
@PhungVanDuy added some comments. I think it is almost ready to merge once the unit test issues are resolved!
I wrote the unit test here can you check it? @ncoop57
@PhungVanDuy added some comments. I think it is almost ready to merge once the unit test issues are resolved!
I wrote the unit test here can you check it? @ncoop57
I just saw your example for the unit test in the main branch. Do you mean I should modify the test code to follow the class style?
@PhungVanDuy it can follow the class style I created or this function style as shown in this pytest site: https://docs.pytest.org/en/7.1.x/getting-started.html
@PhungVanDuy it can follow the class style I created or this function style as shown in this pytest site: https://docs.pytest.org/en/7.1.x/getting-started.html
I just changed the style for tests.py file, let me know if have any problems.
@PhungVanDuy could you grant me access to make changes to this PR?
You can enable it by following this blog: https://github.blog/2016-09-07-improving-collaboration-with-forks/
Okay, so fixed up the tests and pyproject.toml. Sadly I can't actually run the tests since they require S3 keys haha. @reshinthadithyan can you run the tests? I am not on the HPC. If not we can just go ahead and merge. It lgtm
Okay, so fixed up the tests and pyproject.toml. Sadly I can't actually run the tests since they require S3 keys haha. @reshinthadithyan can you run the tests? I am not on the HPC. If not we can just go ahead and merge. It lgtm
Can we merge after today?, I need some edit readme for data processing.
@PhungVanDuy I think it is ready to merge. That okay?
@ncoop57 yes, please
Hey @PhungVanDuy looking good! One thing that will need to be changed though is to use our interfaces for code datasets. You can see an example of how this works with the stackexchange dataset: https://github.com/CarperAI/Code-Pile/blob/main/codepile/stackexchange/stackexchange.py
Essentially, we want to follow the same style for all our datasets so that the code is easier to manage and extend