Skip to content

Conversation

@mlemayTTE
Copy link
Contributor

@mlemayTTE mlemayTTE commented Mar 28, 2025

Closes #58

This PR add tests and type checking in the CI. The work done includes:

  • add pytest in the CI and small corrections when possible or disable failing tests in geos-trame and geos-mesh packages
  • add new github workflow typing-check to include packages that respect typing (geos-posp, geos-timehistory, geos-utils, geos-xml-tools, hdf5-wrapper)
  • linting TTE packages with CI rules

Future work:

  • update disabled tests and remove skip decorator
  • add tests to packages that do not have any
  • update packages where type checking fails (geos-ats, geos-mesh, geos-xml-viewer, geos-trame) and add them to the typing-check workflow
  • add test coverage (need at least one test per package)

mlemayTTE and others added 8 commits March 24, 2025 16:34
@mlemayTTE
Copy link
Contributor Author

@CusiniM if you have a moment, can you check everything is ok at least in geos-ats, geos-mesh and the CI configuration?
@untereiner same for geos-mesh, geos-xml-tools, geos-xml-viewer, geos-trame and the CI configuration?
@alexbenedicto same for geos-mesh, geos-timehistory, TTE packages, and hdf5-wrapper?
@paloma-martinez same for TTE packages?
Thanks all!

@untereiner
Copy link

Is it possible to add this step in the CI. Adding conventional semantics checking will ease packaging. You'll be able to add automatic package numbering for deployment based on the work you are doing now.

@untereiner
Copy link

As a more modern approach I would recommend to keep the mypy section in the pyproject.toml. Mypy will look for it's configuration in this file

@mlemayTTE mlemayTTE changed the title Update CI to include new packages ci: Update CI to include new packages, typing and tests Apr 1, 2025
@mlemayTTE
Copy link
Contributor Author

mlemayTTE commented Apr 1, 2025

As a more modern approach I would recommend to keep the mypy section in the pyproject.toml. Mypy will look for it's configuration in this file

I choose to add a .mypy.ini file to ensure that all the packages follow the same typing rules (similarly to linting approach using the .style.yml file). @untereiner do you mean to define typing (and linting) rules in a pyproject.toml file in the root directory in addition to the pyproject.toml in each package directory?

@untereiner
Copy link

Ok you are right. Not sure it is possible to have a root pyproject only for mypy and linting.

Copy link
Collaborator

@alexbenedicto alexbenedicto left a comment

Choose a reason for hiding this comment

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

After discussing the question of python version in the infrastructure meeting, @CusiniM told me that you could build GEOS with your own python version as long as your pygeos uses the same. So there is no limitation of a specific 3.9 version as I thought. So using 3.10 as a baseline is okay.

@alexbenedicto
Copy link
Collaborator

I noticed in the checks of the docs that some warnings appeared, are these problematic ?
image

@mlemayTTE
Copy link
Contributor Author

mlemayTTE commented Apr 4, 2025

@alexbenedicto The warning in the doc does not prevent building it but I agree it is a bit annoying. I tried to fix it by installing additional packages (from Google research) but it still appeared (this commit).
So I think we can keep working with it since it is not an error and we will try to solve it in the future.

@mlemayTTE
Copy link
Contributor Author

@untereiner do you have any additional comments or are you ok with the PR?

Copy link

@untereiner untereiner left a comment

Choose a reason for hiding this comment

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

LGTM !

@alexbenedicto alexbenedicto self-requested a review April 4, 2025 23:17
@alexbenedicto alexbenedicto merged commit dc0a791 into main Apr 4, 2025
38 of 39 checks passed
@alexbenedicto alexbenedicto deleted the lemay/ci/update_tests_ruff_mypy branch April 4, 2025 23:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Settup Github CI/CD with unit tests, mypy and linting checks

4 participants