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

Add remaining test cases for apply_nan_mode #1108

Open
samarth9008 opened this issue Aug 2, 2024 · 6 comments
Open

Add remaining test cases for apply_nan_mode #1108

samarth9008 opened this issue Aug 2, 2024 · 6 comments
Assignees
@mihir1906

Comments

@samarth9008
Copy link
Collaborator

Function to add test cases for - https://github.com/kaizen-ai/kaizenflow/blob/423470c58122af11b12a1b9c3ea889abcad52342/helpers/hdataframe.py#L163

Few tests are already added over here - https://github.com/kaizen-ai/kaizenflow/blob/423470c58122af11b12a1b9c3ea889abcad52342/helpers/test/test_dataframe.py#L120

Analyze what tests are remaining and add more tests.

See unit test doc to follow the code style
Also highly recommend to read this doc before submitting the PR

FYI @gpsaggese @sonaalKant @DanilYachmenev
@mihir1906
Copy link
Collaborator

mihir1906 commented Aug 7, 2024 •
edited
Loading

(amp.client_venv) mihir@mihir-VMware-Virtual-Platform:~/src/kaizenflow1$ invoke docker_bash
docker pull 623860924167.dkr.ecr.eu-north-1.amazonaws.com/cmamp:dev
Error response from daemon: Head "https://623860924167.dkr.ecr.eu-north-1.amazonaws.com/v2/cmamp/manifests/dev": no basic auth credentials

@samarth9008 @sonaalKant I am getting this error while starting the docker container. Do I need any credentials for accessing the docker container?

@samarth9008
Copy link
Collaborator Author

samarth9008 commented Aug 7, 2024 •
edited
Loading

Let me take a look later.

@samarth9008
Copy link
Collaborator Author

Should work now.

@mihir1906
Copy link
Collaborator

Should work now.

Still getting the same error. Any suggestions on how can I debug this?

@samarth9008
Copy link
Collaborator Author

samarth9008 commented Aug 8, 2024 •
edited
Loading

Did you pull the latest master? After pulling, run this command

source dev_scripts/setenv_amp.sh

@mihir1906
Copy link
Collaborator

It's working now! Thank you.

@mihir1906 mihir1906 mentioned this issue Aug 8, 2024
Merged
samarth9008 pushed a commit that referenced this issue Aug 21, 2024
@mihir1906
KaizenTask1108 adding test cases for apply_nan_mode() #1108 (#1117)
b94c1e2 
* Added test cases for apply_nan_mode.

* Added test cases for apply_nan_mode and fixed the lints.

* Defined correct type for var actual_info.

* catching the error in test7.

* Freeze outputs for the tests 8 and 10 and added comments.

* Added comments.

* Added required comments. Added check for the returned series in test8 and removed test9 and test10.

* Added tests for invalid mode and when mode is strict without nans in the series.

* Added check for the returned series while updating golden outcome for test8.

* Removed test8 to resolve checks on GitHub.

* Added test8 to resolve checks on GitHub.

* outcome updated.

* outcome updated.

* Fixing the suggested nits.

* Delete helpers/test/outcomes/Test_apply_nan_mode.test8/output/test.txt

Removed helpers/test/outcomes/Test_apply_nan_mode.test8/output/test.txt

* Updated all LOCs including comments to within 80 char per line.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mihir1906 mihir1906

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@samarth9008 @mihir1906