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

MNT: Move cartopy feature download script into the package #2263

Merged
merged 1 commit into from
Jan 10, 2024
Merged

MNT: Move cartopy feature download script into the package #2263

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .github/workflows/ci-testing.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,9 @@ jobs:
shell: bash
run: |
# Check that the downloader tool at least knows where to get the data from (but don't actually download it)
python tools/cartopy_feature_download.py gshhs physical --dry-run
python -m cartopy.feature.download gshhs physical --dry-run
# It should also be available as a script
cartopy_feature_download gshhs physical --dry-run
CARTOPY_GIT_DIR=$PWD
pytest -ra -n 4 \
--color=yes \
Expand Down
8 changes: 6 additions & 2 deletions tools/cartopy_feature_download.py → lib/cartopy/feature/download/__main__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

For detail on how to use this tool, execute it with the `-h` option:

python cartopy_feature_download.py -h
python -m cartopy.feature.download -h

"""

Expand Down Expand Up @@ -107,7 +107,7 @@ def download_features(group_names, dry_run=True):
''.format(category, name, scale, len(geoms)))


if __name__ == '__main__':
def main():
parser = argparse.ArgumentParser(description='Download feature datasets.')
parser.add_argument('group_names', nargs='+',
choices=FEATURE_DEFN_GROUPS,
Expand Down Expand Up @@ -146,3 +146,7 @@ def download_features(group_names, dry_run=True):
config['downloaders'][SHP_NE_SPEC].url_template = URL_TEMPLATE

download_features(args.group_names, dry_run=args.dry_run)


if __name__ == '__main__':
main()
2 changes: 1 addition & 1 deletion pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ plotting = ["pillow>=6.1.0", "scipy>=1.3.1"]
test = ["pytest>=5.1.2", "pytest-mpl>=0.11", "pytest-xdist", "pytest-cov", "coveralls"]

[project.scripts]
Copy link
Contributor

Choose a reason for hiding this comment

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

In addition to the Why not leaving it also as executable script that is installed with the package, typically named cartopy_feature_download? Maybe you must declare a main function in the __main__ module for that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I think you're right, we need to define a function that can be called from the script for that. I moved the content in the if block inside a separate main() function like you suggested.

Now you can access this script via: python -m cartopy.feature.download -h or cartopy_feature_download gshhs physical --dry-run

I guess this just brings up the question of whether this installed script is desired or not. I don't have a major preference either way, but would probably slightly lean towards not including it in the scripts section since it is not a critical piece of cartopy. But, it looks like you @stefraynaud were the one that added this as an installation piece, so maybe it is useful for you as a standalone script? Previously, I think it made sense because it was only accessible via a git clone of the repository to access the tools directory, but now you can execute the module which is installed with the package as well.

Copy link
Contributor

@stefraynaud stefraynaud Oct 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Installing these data once for all is useful when used for instance on a computer node that has no internet access.
I had to solve the problem for several people that don't necessarily git clone cartopy.
It is not critical because of cartopy_offlinedata conda package but not everyone has their own python installation to add this package.
Making it easily accessible as an executable is not difficult once the main function is declared. Then we just have to say "execute cartopy_feature_download -h from whereever your are".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree with the first part on this function being useful without git cloning.

My second part was just suggesting to now tell users from wherever you are type: python -m cartopy.feature.download -h

But, I'm fine either way with including the install script, so left it in for now.

feature_download = "tools.cartopy_feature_download.py:__main__"
cartopy_feature_download = "cartopy.feature.download.__main__:main"

[project.urls]
documentation='https://scitools.org.uk/cartopy/docs/latest/'
Expand Down