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

[Fix] Running the unit tests clutters the project directory #1173

Closed
fpgmaas opened this issue Sep 7, 2022 · 8 comments
Closed

[Fix] Running the unit tests clutters the project directory #1173

fpgmaas opened this issue Sep 7, 2022 · 8 comments
Labels
bug Something isn't working

Comments

@fpgmaas
Copy link

fpgmaas commented Sep 7, 2022

Summary

Running the unit tests with make test clutters the project directory. Maybe it's possible to have these files downloaded to a separate directory, and delete them when pytest finishes or aborts? Maybe with the use of try/finally?

Codes

make test
git status

Outputs

?? covid_japan_metadata.csv
?? japan.csv
?? ne_10m_admin_1_states_provinces/

Environment

  • CovsirPhy version: master branch
  • Python version: 3.9
  • Installation: not applicable
  • System: WSL Mac OS
@fpgmaas fpgmaas added the bug Something isn't working label Sep 7, 2022
@fpgmaas fpgmaas changed the title [Fix] Running the unit tests clutters the project directory [Fix] Sep 7, 2022
@lisphilar lisphilar added this to the Release v2.26.2 milestone Sep 7, 2022
@lisphilar lisphilar changed the title Running the unit tests clutters the project directory [Fix] [Fix] Running the unit tests clutters the project directory Sep 7, 2022
@lisphilar
Copy link
Owner

Thank you for raising.
To avoid downloading files many times, I think downloaded files should be saved in "input" directory. With the current .gitignore configuration, this directory is not the target of git statsus. I will fix this with a pull request.

@lisphilar
Copy link
Owner

lisphilar commented Sep 7, 2022

With #1178, ”japan.csv" and "covid_japan_metadata.csv" created with JapanData class and test workflow will be moved to "input" directory from top directory.

Regarding "ne_10m_admin_1_states_provinces/", I have one idea. Because this is from Natural Earth and published under Public Domain, will it be better to include it in covsirphy as geopandas does?

lisphilar added a commit that referenced this issue Sep 7, 2022
test: #1173, move created files with test_japan_data.py to input dir
@fpgmaas
Copy link
Author

fpgmaas commented Sep 8, 2022

I think that sounds like a good solution!

@lisphilar
Copy link
Owner

lisphilar commented Sep 8, 2022

Thank you and I will try to implement it :)

Note:

  • Add a script of downloading/updating the dataset to stable version publish workflow
  • Save the file in covsirphy/gis/natural_earth directory with README.md (to explain the data source)
  • Ensure covsirphy to include the dataset with pyproject.toml (using "include" option?)
  • covsirphy.gis.geometry._Geometry._natural_earth() will read the saved files internally (with relative path from covsirphy directory?)
  • Explain the workflow and data source in documentation

@lisphilar
Copy link
Owner

Because the dataset of the data source will be updated irregularly, the dataset will not be included in CovsirPhy. As a quick solution, the directory to save the files will be changed to "covsirphy/gis/Natural_Earth" directory.

@lisphilar lisphilar mentioned this issue Sep 11, 2022
@lisphilar
Copy link
Owner

@fpgmaas
Solution was changed, but I tried to solve this issue. Any ideas or could you confirm that the files will not be created?

@fpgmaas
Copy link
Author

fpgmaas commented Sep 12, 2022

Can confirm that the files are not showing up in git status after running make test. So the issue seems resolved!

@lisphilar
Copy link
Owner

Thank you for confirmation!
I will close this issue for release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants