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

Group 17: worldfinder #19

Open
11 of 29 tasks
michael-gelfand opened this issue Jan 28, 2025 · 10 comments
Open
11 of 29 tasks

Group 17: worldfinder #19

michael-gelfand opened this issue Jan 28, 2025 · 10 comments

Comments

@michael-gelfand
Copy link

Submitting Author: Michael Gelfand (@michael-gelfand)
All current maintainers: (@michael-gelfand, @BChangs99, @fishydays, @kexins23)
Package Name: worldfinder
One-Line Description of Package: Worldfinder is a package that provides tools to find out information about cities and countries across the world.
Repository Link: https://github.com/UBC-MDS/DSCI524-2425-17-worldfinder
Version submitted: v1.1.0
Editor: TBD
Reviewer 1: Thamer Aldawood
Reviewer 2: Senan Colombe Schellariel Tolokin
Reviewer 3: Rong Wan
Reviewer 4: Fazeeia Mohammed
Archive: TBD
JOSS DOI: TBD
Version accepted: TBD
Date accepted (month/day/year): TBD


Code of Conduct & Commitment to Maintain Package

Description

This packages provides a set of four functions for working with geographical information about cities and countries. These functions will allow users to find the capital city of a country, find all countries that contain a given city name, determine if a city belongs to a specific country, and get statistics about a specified country such as population, GDP, and surface area. These functions will utilize a pre-existing database of city and country information to return the necessary information.

Scope

  • Please indicate which category or categories.
    Check out our package scope page to learn more about our
    scope. (If you are unsure of which category you fit, we suggest you make a pre-submission inquiry):

    • Data retrieval
    • Data extraction
    • Data processing/munging
    • Data deposition
    • Data validation and testing
    • Data visualization1
    • Workflow automation
    • Citation management and bibliometrics
    • Scientific software wrappers
    • Database interoperability

Domain Specific & Community Partnerships

- [ ] Geospatial
- [x] Education
- [ ] Pangeo

Community Partnerships

If your package is associated with an
existing community please check below:

  • For all submissions, explain how the and why the package falls under the categories you indicated above. In your explanation, please address the following points (briefly, 1-2 sentences for each):

    • Who is the target audience and what are scientific applications of this package?

      • Anyone who wants to learn more about geography and looking to find out more about countries.
    • Are there other Python packages that accomplish the same thing? If so, how does yours differ?

      • Similar functions do exist in the country-capitals package, the Countrydetails package, and the geopy, however our function for getting a list of all countries containing a city does not appear to be replicated in any other public packages.
    • If you made a pre-submission enquiry, please paste the link to the corresponding issue, forum post, or other discussion, or @tag the editor you contacted:

Technical checks

For details about the pyOpenSci packaging requirements, see our packaging guide. Confirm each of the following by checking the box. This package:

  • does not violate the Terms of Service of any service it interacts with.
  • uses an OSI approved license.
  • contains a README with instructions for installing the development version.
  • includes documentation with examples for all functions.
  • contains a tutorial with examples of its essential functions and uses.
  • has a test suite.
  • has continuous integration setup, such as GitHub Actions CircleCI, and/or others.

Publication Options

JOSS Checks
  • The package has an obvious research application according to JOSS's definition in their submission requirements. Be aware that completing the pyOpenSci review process does not guarantee acceptance to JOSS. Be sure to read their submission requirements (linked above) if you are interested in submitting to JOSS.
  • The package is not a "minor utility" as defined by JOSS's submission requirements: "Minor ‘utility’ packages, including ‘thin’ API clients, are not acceptable." pyOpenSci welcomes these packages under "Data Retrieval", but JOSS has slightly different criteria.
  • The package contains a paper.md matching JOSS's requirements with a high-level description in the package root or in inst/.
  • The package is deposited in a long-term repository with the DOI:

Note: JOSS accepts our review as theirs. You will NOT need to go through another full review. JOSS will only review your paper.md file. Be sure to link to this pyOpenSci issue when a JOSS issue is opened for your package. Also be sure to tell the JOSS editor that this is a pyOpenSci reviewed package once you reach this step.

Are you OK with Reviewers Submitting Issues and/or pull requests to your Repo Directly?

This option will allow reviewers to open smaller issues that can then be linked to PR's rather than submitting a more dense text based review. It will also allow you to demonstrate addressing the issue via PR links.

  • Yes I am OK with reviewers submitting requested changes as issues to my repo. Reviewers will then link to the issues in their submitted review.

Confirm each of the following by checking the box.

  • I have read the author guide.
  • I expect to maintain this package for at least 2 years and can help find a replacement for the maintainer (team) if needed.

P.S. Have feedback/comments about our review process? Leave a comment here

Editor and Review Templates

The editor template can be found here.

The review template can be found here.

Footnotes

  1. Please fill out a pre-submission inquiry before submitting a data visualization package.

@colombe-tolokin
Copy link

colombe-tolokin commented Jan 29, 2025

Package Review

Please check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide

  • As the reviewer I confirm that there are no conflicts of interest for me to review this work (If you are unsure whether you are in conflict, please speak to your editor before starting your review).

Documentation

The package includes all the following forms of documentation:

  • A statement of need clearly stating problems the software is designed to solve and its target audience in README.
  • Installation instructions: for the development version of the package and any non-standard dependencies in README.
  • Vignette(s) demonstrating major functionality that runs successfully locally.
  • Function Documentation: for all user-facing functions.
  • Examples for all user-facing functions.
  • Community guidelines including contribution guidelines in the README or CONTRIBUTING.
  • [ x] Metadata including author(s), author e-mail(s), a url, and any other relevant metadata e.g., in a pyproject.toml file or elsewhere.

Readme file requirements
The package meets the readme requirements below:

  • Package has a README.md file in the root directory.

The README should include, from top to bottom:

  • The package name
  • Badges for:
    • Continuous integration and test coverage,
    • Docs building (if you have a documentation website),
    • A repostatus.org badge,
    • Python versions supported,
    • Current package version (on PyPI / Conda).

NOTE: If the README has many more badges, you might want to consider using a table for badges: see this example. Such a table should be more wide than high. (Note that the a badge for pyOpenSci peer-review will be provided upon acceptance.)

  • Short description of package goals.
  • Package installation instructions
  • Any additional setup required to use the package (authentication tokens, etc.)
  • Descriptive links to all vignettes. If the package is small, there may only be a need for one vignette which could be placed in the README.md file.
    • [ x] Brief demonstration of package usage (as it makes sense - links to vignettes could also suffice here if package description is clear)
  • Link to your documentation website.
  • If applicable, how the package compares to other similar packages and/or how it relates to other packages in the scientific ecosystem.
  • Citation information

Usability

Reviewers are encouraged to submit suggestions (or pull requests) that will improve the usability of the package as a whole.
Package structure should follow general community best-practices. In general please consider whether:

  • [ x] Package documentation is clear and easy to find and use.
  • [ x] The need for the package is clear
  • [ x] All functions have documentation and associated examples for use
  • [ x] The package is easy to install

Functionality

  • [ x] Installation: Installation succeeds as documented.
  • [ x] Functionality: Any functional claims of the software been confirmed.
  • [ x] Performance: Any performance claims of the software been confirmed.
  • [ x] Automated tests:
    • [ x] All tests pass on the reviewer's local machine for the package version submitted by the author. Ideally this should be a tagged version making it easy for reviewers to install.
    • [ x] Tests cover essential functions of the package and a reasonable range of inputs and conditions.
  • [ x] Continuous Integration: Has continuous integration setup (We suggest using Github actions but any CI platform is acceptable for review)
  • [x ] Packaging guidelines: The package conforms to the pyOpenSci packaging guidelines.
    A few notable highlights to look at:
    • [ x] Package supports modern versions of Python and not End of life versions.
    • [ x] Code format is standard throughout package and follows PEP 8 guidelines (CI tests for linting pass)

For packages also submitting to JOSS

Note: Be sure to check this carefully, as JOSS's submission requirements and scope differ from pyOpenSci's in terms of what types of packages are accepted.

The package contains a paper.md matching JOSS's requirements with:

  • A short summary describing the high-level functionality of the software
  • Authors: A list of authors with their affiliations
  • A statement of need clearly stating problems the software is designed to solve and its target audience.
  • References: With DOIs for all those that have one (e.g. papers, datasets, software).

Final approval (post-review)

  • The author has responded to my review and made changes to my satisfaction. I recommend approving this package.

Estimated hours spent reviewing: 2 hours


Review Comments

Good guys! Here are my thoughts about your package:

  • The package is well-written and well-organized, making it easy to follow.

  • Each function behaves as expected and is easy to understand.

  • The offline approach using local .csv files is a nice touch, making it independent of external APIs.

  • The written code is well-covered by tests, which is great to see.

But I also have suggestions:

  • The README currently provides some examples, but adding more complex scenarios could help users fully explore the package.

  • The check_city() function could be optimized by narrowing down the search to cities within the specified country, which could improve performance on larger datasets.

@ThamerD
Copy link

ThamerD commented Jan 30, 2025

Package Review

  • As the reviewer I confirm that there are no conflicts of interest for me to review this work (If you are unsure whether you are in conflict, please speak to your editor before starting your review).

Documentation

The package includes all the following forms of documentation:

  • A statement of need clearly stating problems the software is designed to solve and its target audience in README.
  • Installation instructions: for the development version of the package and any non-standard dependencies in README.
  • Vignette(s) demonstrating major functionality that runs successfully locally.
  • Function Documentation: for all user-facing functions.
  • Examples for all user-facing functions.
  • Community guidelines including contribution guidelines in the README or CONTRIBUTING.
  • Metadata including author(s), author e-mail(s), a url, and any other relevant metadata e.g., in a pyproject.toml file or elsewhere.

Readme file requirements
The package meets the readme requirements below:

  • Package has a README.md file in the root directory.

The README should include, from top to bottom:

  • The package name

  • Badges for:

    • Continuous integration and test coverage,
    • Docs building (if you have a documentation website),
    • A repostatus.org badge,
    • Python versions supported,
    • Current package version (on PyPI / Conda).
  • Short description of package goals.

  • Package installation instructions

  • Any additional setup required to use the package (authentication tokens, etc.)

  • Descriptive links to all vignettes. If the package is small, there may only be a need for one vignette which could be placed in the README.md file.

    • Brief demonstration of package usage (as it makes sense - links to vignettes could also suffice here if package description is clear)
  • Link to your documentation website.

  • If applicable, how the package compares to other similar packages and/or how it relates to other packages in the scientific ecosystem.

  • Citation information

Usability

Reviewers are encouraged to submit suggestions (or pull requests) that will improve the usability of the package as a whole.
Package structure should follow general community best-practices. In general please consider whether:

  • Package documentation is clear and easy to find and use.
  • The need for the package is clear
  • All functions have documentation and associated examples for use
  • The package is easy to install

Functionality

  • Installation: Installation succeeds as documented.
  • Functionality: Any functional claims of the software been confirmed.
  • Performance: Any performance claims of the software been confirmed.
  • Automated tests:
    • All tests pass on the reviewer's local machine for the package version submitted by the author. Ideally this should be a tagged version making it easy for reviewers to install.
    • Tests cover essential functions of the package and a reasonable range of inputs and conditions.
  • Continuous Integration: Has continuous integration setup (We suggest using Github actions but any CI platform is acceptable for review)
  • Packaging guidelines: The package conforms to the pyOpenSci packaging guidelines.
    A few notable highlights to look at:
    • Package supports modern versions of Python and not End of life versions.
    • Code format is standard throughout package and follows PEP 8 guidelines (CI tests for linting pass)

Final approval (post-review)

  • The author has responded to my review and made changes to my satisfaction. I recommend approving this package.

Estimated hours spent reviewing: 2 hours


Review Comments

Very useful package that I may even use myself in the future. The code is well written and documented. Well done!

I do have some minor issues that will hopefully provide opportunities for improvement:

  1. Pytest fails when running "test_load_data_functionality()".
    Image
  2. Some functions don't have the "raises" section in their docstring whereas others do.
  3. Some functions don't accept spaces in inputs whereas others do.
    1. For example, get_capital("Fiji") works but get_country_statistic("Fiji ", "gdp") raises an error.
  4. Is there a valid reason for your "check_city()" function to look through the entire cities dataset with every call? I worry this will cause slowdowns with repeated calls as the city dataset is fairly large. Wouldn't it be better to only search through the list of cities within the given country and compare those with the input?
  5. I understand it's not strictly within your package's scope, but I do think it's important to at least acknowledge the following issues:
    1. Some countries that are currently fighting for recognition are missing from the dataset (e.g., Taiwan, Palestine, Kosovo).
    2. Some countries contain multiple capitals (either due to conflict or their government structure). https://en.wikipedia.org/wiki/List_of_countries_with_multiple_capitals
  6. I see you mentioned where you got your data from, but it would be useful to know where the root source of the data actually is. Especially when considering the credibility of certain statistics such as "unemployment rate".

@BChangs99
Copy link

BChangs99 commented Jan 30, 2025

Thanks @ThamerD for taking the time to review our package!

We've compiled yours and any further feedback into a big issue board here: UBC-MDS/DSCI524-2425-17-worldfinder#65

As we move forward tackling these, if there's any feedback we don't fully agree with and fix, we'll provide an explanation on our perspective.

Genuinely appreciate the feedback!

Team 17

@OliviaWan56
Copy link

Package Review

Please check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide

  • [√ ] As the reviewer I confirm that there are no conflicts of interest for me to review this work (If you are unsure whether you are in conflict, please speak to your editor before starting your review).

Documentation

The package includes all the following forms of documentation:

  • [√ ] A statement of need clearly stating problems the software is designed to solve and its target audience in README.
  • [√ ] Installation instructions: for the development version of the package and any non-standard dependencies in README.
  • [√ ] Vignette(s) demonstrating major functionality that runs successfully locally.
  • [√ ] Function Documentation: for all user-facing functions.
  • [√ ] Examples for all user-facing functions.
  • [√ ] Community guidelines including contribution guidelines in the README or CONTRIBUTING.
  • [√ ] Metadata including author(s), author e-mail(s), a url, and any other relevant metadata e.g., in a pyproject.toml file or elsewhere.

Readme file requirements
The package meets the readme requirements below:

  • [√] Package has a README.md file in the root directory.

The README should include, from top to bottom:

  • [√] The package name
  • [√] Badges for:
    • [√] Continuous integration and test coverage,
    • [√] Docs building (if you have a documentation website),
    • [√] A repostatus.org badge,
    • [√] Python versions supported,
    • [√] Current package version (on PyPI / Conda).

NOTE: If the README has many more badges, you might want to consider using a table for badges: see this example. Such a table should be more wide than high. (Note that the a badge for pyOpenSci peer-review will be provided upon acceptance.)

  • [√ ] Short description of package goals.
  • [√] Package installation instructions
  • [√ ] Any additional setup required to use the package (authentication tokens, etc.)
  • [√ ] Descriptive links to all vignettes. If the package is small, there may only be a need for one vignette which could be placed in the README.md file.
    • [√ ] Brief demonstration of package usage (as it makes sense - links to vignettes could also suffice here if package description is clear)
  • [√ ] Link to your documentation website.
  • If applicable, how the package compares to other similar packages and/or how it relates to other packages in the scientific ecosystem.
  • Citation information

Usability

Reviewers are encouraged to submit suggestions (or pull requests) that will improve the usability of the package as a whole.
Package structure should follow general community best-practices. In general please consider whether:

  • [√ ] Package documentation is clear and easy to find and use.
  • [√ ] The need for the package is clear
  • [√ ] All functions have documentation and associated examples for use
  • [√ ] The package is easy to install

Functionality

  • [√ ] Installation: Installation succeeds as documented.
  • [√ ] Functionality: Any functional claims of the software been confirmed.
  • [√ ] Performance: Any performance claims of the software been confirmed.
  • [√ ] Automated tests:
    • [√ ] All tests pass on the reviewer's local machine for the package version submitted by the author. Ideally this should be a tagged version making it easy for reviewers to install.
    • [√ ] Tests cover essential functions of the package and a reasonable range of inputs and conditions.
  • [√ ] Continuous Integration: Has continuous integration setup (We suggest using Github actions but any CI platform is acceptable for review)
  • [√ ] Packaging guidelines: The package conforms to the pyOpenSci packaging guidelines.
    A few notable highlights to look at:
    • [ √] Package supports modern versions of Python and not End of life versions.
    • [√ ] Code format is standard throughout package and follows PEP 8 guidelines (CI tests for linting pass)

For packages also submitting to JOSS

  • [√ ] The package has an obvious research application according to JOSS's definition in their submission requirements.

Note: Be sure to check this carefully, as JOSS's submission requirements and scope differ from pyOpenSci's in terms of what types of packages are accepted.

The package contains a paper.md matching JOSS's requirements with:

  • [√ ] A short summary describing the high-level functionality of the software
  • [√ ] Authors: A list of authors with their affiliations
  • [√ ] A statement of need clearly stating problems the software is designed to solve and its target audience.
  • [√ ] References: With DOIs for all those that have one (e.g. papers, datasets, software).

Final approval (post-review)

  • The author has responded to my review and made changes to my satisfaction. I recommend approving this package.

Estimated hours spent reviewing: 2hours


Review Comments

After reviewing the tests directory of the worldfinder repository, here are some suggestions for potential improvements:
Comprehensive Testing: Ensure that all functions within the worldfinder package are thoroughly tested. This includes edge cases, such as:
Non-existent city or country names.
Case sensitivity issues.
Empty string inputs.
Unexpected data types.

Here are some aspects that the worldfinder repository does well:
Presence of Unit Tests: The repository has a structured tests directory, which indicates a focus on test-driven development (TDD).
The presence of test scripts ensures that core functionalities are being validated.
Use of pytest: The tests appear to be written using pytest, which is a widely used and efficient testing framework for Python.
This allows for flexible test execution and better debugging.

These strong points contribute to the maintainability and robustness of the worldfinder package. With minor improvements, such as additional test coverage and better organization, the testing framework could be further enhanced.

@mindy001
Copy link

mindy001 commented Jan 31, 2025

Package Review

Please check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide

  • [x ] As the reviewer I confirm that there are no conflicts of interest for me to review this work (If you are unsure whether you are in conflict, please speak to your editor before starting your review).

Documentation

The package includes all the following forms of documentation:

  • [x ] A statement of need clearly stating problems the software is designed to solve and its target audience in README.
  • [x ] Installation instructions: for the development version of the package and any non-standard dependencies in README.
  • [x ] Vignette(s) demonstrating major functionality that runs successfully locally.
  • [x ] Function Documentation: for all user-facing functions.
  • [x ] Examples for all user-facing functions.
  • [x ] Community guidelines including contribution guidelines in the README or CONTRIBUTING.
  • [x ] Metadata including author(s), author e-mail(s), a url, and any other relevant metadata e.g., in a pyproject.toml file or elsewhere.

Readme file requirements
The package meets the readme requirements below:

  • [x ] Package has a README.md file in the root directory.

The README should include, from top to bottom:

  • [ x] The package name
  • [ x] Badges for:
    • [x ] Continuous integration and test coverage,
    • [x ] Docs building (if you have a documentation website),
    • [x ] A repostatus.org badge,
    • [x ] Python versions supported,
    • [x ] Current package version (on PyPI / Conda).

NOTE: If the README has many more badges, you might want to consider using a table for badges: see this example. Such a table should be more wide than high. (Note that the a badge for pyOpenSci peer-review will be provided upon acceptance.)

  • [x ] Short description of package goals.
  • [x ] Package installation instructions
  • [x ] Any additional setup required to use the package (authentication tokens, etc.)
  • [x ] Descriptive links to all vignettes. If the package is small, there may only be a need for one vignette which could be placed in the README.md file.
    • [x ] Brief demonstration of package usage (as it makes sense - links to vignettes could also suffice here if package description is clear)
  • Link to your documentation website.
  • If applicable, how the package compares to other similar packages and/or how it relates to other packages in the scientific ecosystem.
  • Citation information

Usability

Reviewers are encouraged to submit suggestions (or pull requests) that will improve the usability of the package as a whole.
Package structure should follow general community best-practices. In general please consider whether:

  • [x ] Package documentation is clear and easy to find and use.
  • [x ] The need for the package is clear
  • [x ] All functions have documentation and associated examples for use
  • The package is easy to install

Functionality

  • [x ] Installation: Installation succeeds as documented.
  • [x ] Functionality: Any functional claims of the software been confirmed.
  • [x ] Performance: Any performance claims of the software been confirmed.
  • [x ] Automated tests:
    • [x ] All tests pass on the reviewer's local machine for the package version submitted by the author. Ideally this should be a tagged version making it easy for reviewers to install.
    • [ x] Tests cover essential functions of the package and a reasonable range of inputs and conditions.
  • [x ] Continuous Integration: Has continuous integration setup (We suggest using Github actions but any CI platform is acceptable for review)
  • [x ] Packaging guidelines: The package conforms to the pyOpenSci packaging guidelines.
    A few notable highlights to look at:
    • Package supports modern versions of Python and not End of life versions.
    • Code format is standard throughout package and follows PEP 8 guidelines (CI tests for linting pass)

For packages also submitting to JOSS

Note: Be sure to check this carefully, as JOSS's submission requirements and scope differ from pyOpenSci's in terms of what types of packages are accepted.

The package contains a paper.md matching JOSS's requirements with:

  • [x ] A short summary describing the high-level functionality of the software
  • [x ] Authors: A list of authors with their affiliations
  • A statement of need clearly stating problems the software is designed to solve and its target audience.
  • [x ] References: With DOIs for all those that have one (e.g. papers, datasets, software).

Final approval (post-review)

  • The author has responded to my review and made changes to my satisfaction. I recommend approving this package.

Estimated hours spent reviewing:


Review Comments

What was done well:

  1. Loading Data from CSV: Your load_data function ensures that the file exists and handles both relative and absolute paths. It looks well thought out.

  2. Error Handling: The code in each function seems to include good error checking (type checks, empty string checks, etc.). That helps make your functions more robust.

What can be improved on :

  1. Whitespace Handling: Testing for leading/trailing spaces ensures your functions will work even with imperfect input from users. Additionally what happens when other characters are accidently added. like"#@(#$&" Special characters.

  2. Test get_capital_multiple_capitals: You mentioned that South Africa has multiple capitals, but your assertion checks if the capital returned is in the list of expected capitals (["Pretoria", "Cape Town"]). However, get_capital might return just one capital, and this might be a clarification point. You could modify the test to assert the result contains one of the capitals, rather than comparing it to both.

3.Invalid CSV Structure: Simulate situations where the columns are malformed (e.g., missing Capital/Major City or Country) to see if your functions handle it gracefully.

  1. Test Function Documentation: Though you have great docstrings in your test functions, try to ensure that each test case also has comments to explain the reasoning behind the tests. This can be useful for anyone reading the code in the future, especially when debugging failing tests

@michael-gelfand
Copy link
Author

@ThamerD I wanted to address the constructive feedback you provided and let you know the changes we've made to the package!

  1. Regarding the failing pytests, it appeared to be caused by some pathing issues related to our country and city csv's being in a different location than our csv's used for unit testing. Elaine was able to fix that issue and here is a screenshot showing that all our pytest's pass now!

Image

  1. We've updated our docstrings to include the error raising section for each function.
  2. Brian added additional logic in the get_country_statistic function to properly handle leading and trailing whitespace in the arguments.
  3. Regarding the efficiency of the check_city function, we feel that it's current state is optimal. As of now, whenever the function is called, it loads the cities.csv table in as a dataframe, and then filters down that dataframe for only cities that exist in that specified country. The only way I could see efficiency being improved further is by offloading the cities dataset from the package and instead using API calls to grab the data from an online source. Currently we feel that is beyond the scope of our package but may explore it in the future.
  4. Coco has made changes to the project's README to acknowledge that our package has limitations regarding disputed status. As of now implementing changes to address these disputes are beyond the scope of our package but once again, we may address it in the future if we have time.
  5. Coco updated the project's README to include more information on the source of the data so that user's can have a better understanding of where it's coming from.

Additionally, I have gone ahead and added the missing badges to our package's README as I noticed that you indicated we were missing a few. Here is what the top of our README looks like now with these badges added:

Image

@ThamerD
Copy link

ThamerD commented Feb 3, 2025

Thank you for the detailed update @michael-gelfand, I updated my review accordingly.

Well done lads!

@shikexi6
Copy link

shikexi6 commented Feb 3, 2025

@OliviaWan56 Hi Olivia, thank you so much for giving us feedback. I wanted to let you know that we have already ensured our test cases covered all aspects you listed. You can refer to https://github.com/UBC-MDS/DSCI524-2425-17-worldfinder/issues/67. We appreciate your feedback

Team 17

@shikexi6
Copy link

shikexi6 commented Feb 3, 2025

@mindy001 Thank you so much for the feedback! They are really useful for making our package better! We have addressed all of them and please refer to https://github.com/UBC-MDS/DSCI524-2425-17-worldfinder/issues/65 for details. Please don't hesitate to contact us if you have more feedbacks!

Team 17

@fishydays
Copy link

fishydays commented Feb 3, 2025

Hi @colombe-tolokin, thank you for your review of our package. Here is what we changed in response to your suggestions:

  1. Regarding adding more complex scenarios to our README. Our team believes that the examples in the README currently is sufficient for the concise nature of a README file. However, we agree that it will be beneficial for users to be able to access examples that are more complex so they can fully explore the package usage. Hence, we have updated our README to provided a direct link to our tutorial.
  2. Regarding the optimization of the check_city() function. Please refer to point 4 in @michael-gelfand's reply above.
  1. Regarding the efficiency of the check_city function, we feel that it's current state is optimal. As of now, whenever the function is called, it loads the cities.csv table in as a dataframe, and then filters down that dataframe for only cities that exist in that specified country. The only way I could see efficiency being improved further is by offloading the cities dataset from the package and instead using API calls to grab the data from an online source. Currently we feel that is beyond the scope of our package but may explore it in the future.

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

No branches or pull requests

8 participants