aspectumapp / osm2geojson

Convert OSM and Overpass XML/JSON to GeoJSON
MIT License
100 stars 14 forks source link

Created a CLI for the package, made 'area_keys' and 'polygon_features' variable #32

Closed CrsiX closed 2 years ago

CrsiX commented 2 years ago

Referencing #25

This PR adds a command-line interface to the osm2geojson package. It should be executed as python3 -m osm2geojson. Additionally, it allows the area_keys and polygon_features to be set as optional arguments (they are just carried around with in most of the functions).

I've marked this is a draft since I first wanted to get some feedback and suggestions for improvement from you, @rapkin :)

CrsiX commented 2 years ago

Also, to be confident about our work, we need to add tests for cli tool as well (parsing of agruments). What you think about this?

Testing the parsing of arguments is out of scope for this. I would refer to argparse and the wide use of that package. I agree that test coverage is really great, but that should stop at something like CLI argument parsing. After all, the use of the CLI is optional, since it only provides a wrapper around the functionality provided by the library. Therefore, the library should have good test coverage, but not the CLI.

I would instead suggest to add a new entry in the GitHub Actions Workflow that builds the pip package and then installs & runs it locally once (just to be sure it works as intended).

CrsiX commented 2 years ago

I would suggest to add something like this to the end of .github/workflows/pythonpackagetest.yml:

- name: Build the package
  run: |
    pip install setuptools wheel
    python setup.py sdist bdist_wheel
- name: Install the local package and execute it
  run: |
    pip install ./dist/osm2geojson-*.tar.gz
    python -m osm2geojson --help
    osm2geojson --help

It ensures that the pip package can be installed successfully and that the module as well as the script osm2geojson can be executed. Additionally, one may add the following to ensure the same inside a virtual environment:

- name: Install the local package in a venv and execute it
  run: |
    sudo apt install python3-venv -y
    python3 -m venv venv
    . venv/bin/activate
    pip install ./dist/osm2geojson-*.tar.gz
    test -f venv/bin/osm2geojson
    python -m osm2geojson --help
    osm2geojson --help
rapkin commented 2 years ago

I agree, we can add new step to already existing action (to test installation of cli tool). But I think that It's good to have some tests over cli tool. Tests are showing how to use our package + additional layer of validation. I can write those tests after merge, no problem. Your work is awesome, thank you!

rapkin commented 2 years ago

I'm trying to install package on my computer

> python setup.py sdist bdist_wheel
> pip install ./dist/osm2geojson-*.tar.gz
...
ERROR: For req: osm2geojson==0.1.32. Invalid script entry point: <ExportEntry osm2geojson = osm2geojson.__main__.main:None []> - A callable suffix is required. Cf https://packaging.python.org/specifications/entry-points/#use-for-scripts for more information.

I changed the code a bit to 'console_scripts': ['osm2geojson=osm2geojson.__main__.main:main'] and it works.

Maybe you can add this small change and I'll approve your PR. Thank you again!

CrsiX commented 2 years ago

Thank you :)

epsawan commented 1 year ago

I want to get separate object of point, linestrime and polygon. How can I do that

epsawan commented 1 year ago

I want tags out like in general format : tags : { name:... other tags.... }

but I want like this: (don;t want tags object) name:.... other tags...