search

mdp0023 / SVInsight

SVInsight: A python package for calculating an exploratory social vulnerability index (SVI).
https://mdp0023.github.io/SVInsight/
Other
6 stars 1 forks source link

OS- and API-key dependencies in examples #9

Open dataspider opened 3 weeks ago

dataspider commented 3 weeks ago

Please make the examples independent of your operating system and your API key.

You have lines like file_path = "/Users/matthewpreisser/Documents/Research/Codes/SVInsight" (looks like MacOS) in svinsight_example_Harris_County.py or file_path = "C:/Users/pp7545/Documents/research/SVInsight" (looks like Windows) in SVInsight_example_SETx.ipynb (you might also want to include jupyterlab in the dev dependencies, btw). Instead of those, programmatically generate the absolute paths to match the OS your code is run on or tell people to create some kind of .env file to specify your root and read the env via a suitable library.

Regarding the API key: You are redistributing the census data for two countries with the repository, so it seems that you could easily use these data to demonstrate your functionality without making API calls (and then just have an API call demo in a separate example). I don't want to have to get an API key just to try out some software (I know that many people working in your field will have one anyway, but relying on an API key in the examples is unnecessary, so why not make the package more accessible?). Please also check the data licensing situation for the data that you are redistributing, and include the applicable licensing terms in a DATALICENSE file or as a separate section of your license file.

https://github.com/openjournals/joss-reviews/issues/7212

mdp0023 commented 2 weeks ago

Thank you for the suggestion and I agree this will increase the accessibility of the package for first time users. Since the data download functions won’t redownload the input data with the default overwrite parameter (overwrite=False), and as you mentioned the data is provided with the package, the examples should work now without requiring a current API. I also removed the file paths and made them absolute paths, thank you for catching that. The appropriate language has also been added to the license with regard to the included data.