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

create titiler namespaces packages #284

Merged
merged 18 commits into from
Apr 19, 2021
Merged

create titiler namespaces packages #284

merged 18 commits into from
Apr 19, 2021

Conversation

vincentsarago
Copy link
Member

@vincentsarago vincentsarago commented Mar 30, 2021

ref: #278

Why

As mentioned in #278, TiTiler is much more than an application, it's a python module that lets user create custom Tiler.

I believe that splitting core and application could help users to have a better understanding but also help us to move quickly on some features.

How

  • titiler.core: core functions (e.g tiler factories)

Tiler factories, ressources and models

  • titiler.application: demo application

basic application (adds Uvicorn, python-dotenv and brotli dependencies)

main changes

  • split core functionalities and demo application
  • move custom (colormap, tms) to the demo application
  • removed unused function titiler.utils.get_hash
  • switch to pkg_resources to locate templates files
other
  • use a bash script instead of tox to do PyPI publishing

To Do

  • talk
  • update docs
  • update readmes

)
) -> TileMatrixSet:
"""TileMatrixSet Dependency."""
return tms.get(TileMatrixSetId.name)
Copy link
Member Author

Choose a reason for hiding this comment

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

I moved all custom code to the application part

max-complexity = 12
max-line-length = 90

[mypy]
Copy link
Member Author

Choose a reason for hiding this comment

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

moved the linters config to setup.cfg

]
extra_reqs = {
"test": ["pytest", "pytest-cov", "pytest-asyncio", "requests"],
"server": ["uvicorn[standard]>=0.12.0,<0.14.0"],
Copy link
Member Author

Choose a reason for hiding this comment

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

Uvicorn is optional (e.g when running the application in AWS Lambda you don't need Uvicorn)


from geojson_pydantic.features import Feature


def get_hash(**kwargs: Any) -> str:
Copy link
Member Author

Choose a reason for hiding this comment

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

this function wasn't used in titiler

skip_install = true
commands_pre =
python -m pip install titiler/core
python -m pip install titiler/application
Copy link
Member Author

Choose a reason for hiding this comment

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

because titiler-application depends on titiler-core, we install them manually

tox.ini Outdated Show resolved Hide resolved
@vincentsarago
Copy link
Member Author

vincentsarago commented Apr 6, 2021

Still awaiting for feedback but if we move forward with namespaces here is IMO what we could do:

Maybe

cc @geospatial-jeff @kylebarron

@vincentsarago vincentsarago marked this pull request as ready for review April 12, 2021 20:18
@kylebarron
Copy link
Member

Quick notes from chat

  • Decide whether to use - or . in PyPI names, i.e. pip install titiler.core or pip install titiler-core. (AWS CDK uses ., Google uses -, I use .)
  • Should follow up on whether you can use mypy in pre-commit still after moving to namespaces. In my experience, any linters/code formatters that are purely static are a good fit for pre-commit (black, isort, yapf, pylint, etc). If the tool needs to follow imports (mypy, pytest), then it's simplest to run in each module individually.
  • Ideally put all config in config files, e.g. mypy.ini, setup.cfg, pyproject.toml (my preferred, since it's a PEP standard). Then whether run inside or outside pre-commit should give the same output.

scripts/publish Outdated Show resolved Hide resolved
setup.cfg Show resolved Hide resolved
setup.cfg Outdated Show resolved Hide resolved
@vincentsarago vincentsarago changed the title [proposal] create titiler namespaces packages create titiler namespaces packages Apr 13, 2021
@vincentsarago vincentsarago merged commit 6e589cf into master Apr 19, 2021
@vincentsarago vincentsarago deleted the namespacesPackages branch April 19, 2021 15:14
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

Successfully merging this pull request may close these issues.

2 participants