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

Add tests #10

Merged
merged 20 commits into from
Jul 25, 2023
Merged

Add tests #10

merged 20 commits into from
Jul 25, 2023

Conversation

geek-yang
Copy link
Member

We add tests to zampy.datasets functions and functions in utils.

@geek-yang geek-yang marked this pull request as ready for review July 6, 2023 21:11
Copy link
Member Author

@geek-yang geek-yang left a comment

Choose a reason for hiding this comment

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

To review this PR, just check the tests. Note that since functions using xesmf relying on conda/mamba environment, therefore tests related to those functions are conditional (but they are tested under the mamba CD/CI action).

@geek-yang
Copy link
Member Author

Note that CD/CI on windows fails due to an unexpected PermissionError. This post seems to provide a solution to fix it (thanks for suggesting it @SarahAlidoost). Still needs to test whether it works or not.

@geek-yang geek-yang mentioned this pull request Jul 16, 2023
5 tasks
tests/test_regrid.py Outdated Show resolved Hide resolved
tests/test_regrid.py Outdated Show resolved Hide resolved
tests/test_regrid.py Outdated Show resolved Hide resolved
tests/test_regrid.py Outdated Show resolved Hide resolved
Copy link
Member

@SarahAlidoost SarahAlidoost left a comment

Choose a reason for hiding this comment

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

@geek-yang nice work on adding tests 👍 The tests cover most of the codes, well done. 🚀 Thank you for setting up the sonarcloud, too. I left some comments, please let me know if something is unclear. Besides those comments, I am wondering if we can re-organize files e.g. now there is a utils.py file and there is a directory utils. This is confusing. Can utils.py be moved to utils folder?

@sonarcloud
Copy link

sonarcloud bot commented Jul 24, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@geek-yang
Copy link
Member Author

@geek-yang nice work on adding tests 👍 The tests cover most of the codes, well done. 🚀 Thank you for setting up the sonarcloud, too. I left some comments, please let me know if something is unclear. Besides those comments, I am wondering if we can re-organize files e.g. now there is a utils.py file and there is a directory utils. This is confusing. Can utils.py be moved to utils folder?

Thanks a lot for your review @SarahAlidoost. The comments help a lot. I think I have addressed all of them, please take a look again.

Regarding the utils, I totally agree with you 😂. During the development I always open the wrong file or adding the wrong path due to this confusing name. Let's make an issue and change it in another PR.

Copy link
Member

@SarahAlidoost SarahAlidoost left a comment

Choose a reason for hiding this comment

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

@geek-yang thank you for addressing the comments 👍

@geek-yang geek-yang merged commit d673d17 into main Jul 25, 2023
@geek-yang geek-yang deleted the add_tests branch July 25, 2023 08:36
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.

2 participants