brey / pyposeidon Goto Github PK

With numpy 1.20+, the following lines throw warnings similar to these:

DeprecationWarning: `np.str` is a deprecated alias for the builtin `str`. To silence this warning, use `str` by itself. Doing this will not modify any behavior and is safe. If you specifically wanted the numpy scalar type, use `np.str_` here.

/home/runner/work/pyPoseidon/pyPoseidon/pyPoseidon/schism.py:605
/home/runner/work/pyPoseidon/pyPoseidon/pyPoseidon/schism.py:187
/home/runner/work/pyPoseidon/pyPoseidon/pyPoseidon/schism.py:593
/home/runner/work/pyPoseidon/pyPoseidon/pyPoseidon/schism.py:599
/home/runner/work/pyPoseidon/pyPoseidon/pyPoseidon/d3d.py:509
/home/runner/work/pyPoseidon/pyPoseidon/pyPoseidon/d3d.py:778
/home/runner/work/pyPoseidon/pyPoseidon/pyPoseidon/d3d.py:784
/home/runner/work/pyPoseidon/pyPoseidon/pyPoseidon/d3d.py:790

https://numpy.org/devdocs/release/1.20.0-notes.html#deprecations

pygeos is now supported by Geopandas. Check whether we can simplify the code by implementing only geopandas.

The output NetCDF files are not compressed. This can easily be checked with source:

ncdump -s -h ../path/to/output.nc | grep -i deflate

Creating compressed netcdfs will greatly reduce the file size and is something we should consider.

The [tool.poetry] section of pyproject.toml describes the packages's metadata and needs to be updated.

The keys that can be used and their description can be found here: https://python-poetry.org/docs/pyproject/

I think it would be really useful if the netcdf output file was documented (variables, dimensions etc). If the solvers produce output files with different formats then we should document what each solver produces separately. As soon as things are relatively stable, changes in the output format should require a version bump.

The documentation can be either an ipython notebook or a wiki page (or both! :P).

This should only require to add an additional file in .github/workflows/.

Check whether the bathymetry matches the coastlines given.

Users should be able to install pyPoseidon without having to install all the visualization libraries.

There is a weird issue with grids around the international line where SCHISM report some elements as having negative area. It was working before.

Check upstream to SCHISM for something that has changed.

See test_schism_total.

cfgrib is pinned to 0.9.8.5 for now. See how to move pass that.

Follow pydata/xarray#5150

The best python framework is pytest. In order to make usage of its test runner the test suite should be structured according to its own conventions (which we already do).

Nevertheless, it is also a good idea to keep the test suite outside the proper package (docs). Among other things, this makes it easier to grep the source code. E.g.

# search for `ncores` in `pyPoseidon` and `tests` respectively.
ag ncores pyPoseidon
ag ncores tests

I will make a pull request that moves the tests to a separate tests directory. @brey even if the CI passes do not merge this before making sure that you don't have changes in the tests in your local repo. If you do merge it you will have to deal with the conflicts afterwards.

The latest proj version breaks the installation of cartopy on Linux (conda + venv). We need to pin the version of proj to <8.

dem.nc is currently located within tests/data. nevertheless it is also being used in jigsaw.py:

If it is not part of the tests I think we should consider moving it to misc.

This is the file in question
https://github.com/brey/pyPoseidon/blob/4ab9963dc1f2f5c59a07cba0e948c7560ebd24f8/pyPoseidon/tests/data/dem.nc

Follow pydap/pydap#211 to check when pydap > 3.2.2 will solve the problem.

As soon as we are satisfied with the github actions setup, we could consider removing the azure pipelines from the project

ideally, we should keep both so that we are not coupled to a specific service, but we probably lack the resources to maintain both at the same time.

This is mainly for integration with the output of WRF. This can be modified later with merging the 2 workflows

Hardcoding the number of cores is suboptimal. It makes it difficult to develop on low end machines (up to 4 cores) and is wasting time on higher end machines (4+ cores are available).

A reasonable default is to use the maximum number of cores minus one or two. E.g.

import multiprocessing

NCORES = max(1, multiprocessing.cpu_count() - 1)

Alternatively, the number of cores could be specified via an ENV variable, but not sure that something like this is that much useful at the moment.

The files that need to be updated can be found with:

ag --python ncores

add a corresponding meteo subclass