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

Add opentelemac #24529

Merged
merged 8 commits into from
May 26, 2024
Merged

Add opentelemac #24529

merged 8 commits into from
May 26, 2024

Conversation

nicogodet
Copy link
Contributor

@nicogodet nicogodet commented Nov 18, 2023

Checklist

  • Title of this PR is meaningful: e.g. "Adding my_nifty_package", not "updated meta.yaml".
  • License file is packaged (see here for an example).
  • Source is from official source.
  • Package does not vendor other packages. (If a package uses the source of another package, they should be separate packages or the licenses of all packages need to be packaged).
  • If static libraries are linked in, the license of the static library is packaged.
  • Package does not ship static libraries. If static libraries are needed, follow CFEP-18.
  • Build number is 0.
  • A tarball (url) rather than a repo (e.g. git_url) is used in your recipe (see here for more details).
  • GitHub users listed in the maintainer section have posted a comment confirming they are willing to be listed there.
  • When in trouble, please check our knowledge base documentation before pinging a team.

@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipes/opentelemac) and found some lint.

Here's what I've got...

For recipes/opentelemac:

  • The recipe must have some tests.
  • Selectors are suggested to take a <two spaces>#<one space>[<expression>] form. See lines [19, 36, 67]
  • license_file entry is missing, but is required.

For recipes/opentelemac:

  • Recipes should usually depend on matplotlib-base as opposed to matplotlib so that runtime environments do not require large packages like qt.
  • License is not an SPDX identifier (or a custom LicenseRef) nor an SPDX license expression.

Documentation on acceptable licenses can be found here.

@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipes/opentelemac) and found some lint.

Here's what I've got...

For recipes/opentelemac:

  • The recipe must have some tests.

For recipes/opentelemac:

  • License is not an SPDX identifier (or a custom LicenseRef) nor an SPDX license expression.

Documentation on acceptable licenses can be found here.

@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipes/opentelemac) and found some lint.

Here's what I've got...

For recipes/opentelemac:

  • The recipe must have some tests.

@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/opentelemac) and found it was in an excellent condition.

@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipes/opentelemac) and found some lint.

Here's what I've got...

For recipes/opentelemac:

  • If python is a host requirement, it should be a run requirement.
  • Non noarch packages should have python requirement without any version constraints.

@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipes/opentelemac) and found some lint.

Here's what I've got...

For recipes/opentelemac:

  • Non noarch packages should have python requirement without any version constraints.
  • Non noarch packages should have python requirement without any version constraints.

@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/opentelemac) and found it was in an excellent condition.

Copy link

Hi! Thanks for your contribution to conda-forge.
Unfortunately, the recipe was added directly in the recipes folder without its own subfolder.
Please move the recipe file into a folder with the name of the package you want to submit.

For example: if your recipe is currently under recipes/<your_package>.yaml, it should be moved to recipes/<your_package>/meta.yaml.
Thanks!

@nicogodet
Copy link
Contributor Author

@tomsail If you want to look at it...

@nicogodet
Copy link
Contributor Author

@conda-forge-admin, please ping conda-forge/help-python

Because of a bug in f2py, we need for this package to use either numpy <=1.21 or numpy >=1.25 with provided patch.
I can't figure out the correct syntax in meta.yaml ou conda_build_config.yaml to respect these conditions.

@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-webservice.

I was asked to ping @conda-forge/help-python and so here I am doing that.

@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipes/opentelemac) and found some lint.

Here's what I've got...

For recipes/opentelemac:

  • Non noarch packages should have python requirement without any version constraints.
  • Non noarch packages should have python requirement without any version constraints.

@tomsail
Copy link
Contributor

tomsail commented Nov 19, 2023

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipes/opentelemac) and found some lint.

Here's what I've got...

For recipes/opentelemac:

  • Non noarch packages should have python requirement without any version constraints.
  • Non noarch packages should have python requirement without any version constraints.

I will try this and let you know if it works

In meta.yaml:

requirements:
  host:
    - python
    - numpy 1.21.* | >=1.25
  run:
    - python
    - numpy 1.21.* | >=1.25

@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/opentelemac) and found it was in an excellent condition.

@tomsail
Copy link
Contributor

tomsail commented Nov 19, 2023

ok I have just seen your conda_build_config.yaml, ignore my comment above

@@ -85,7 +85,7 @@ outputs:
- matplotlib-base
- mpi4py
- numpy >=1.25
- python >=3.9
- python >=3.9,<3.12
Copy link
Contributor

Choose a reason for hiding this comment

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

why wouldn't we include 3.12?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, I don't think this is a priority on the opentelemac roadmap atm..

- {{ compiler('fortran') }}
- {{ compiler('c') }}
- {{ compiler('cxx') }}
- openmpi
Copy link
Contributor

Choose a reason for hiding this comment

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

we could also keep

        - {{ mpi }} 
        - {{ mpi }}-mpifort 

to have more compiling possibilities

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 but let's do this once it's merged in feedstock

source:
# - url: https://gitlab.pam-retd.fr/otm/telemac-mascaret/-/archive/{{ version }}/telemac-mascaret-{{ version }}.tar.gz
- url: https://github.com/nicogodet/otm-sources/releases/download/{{ version }}/telemac-mascaret-{{ version }}.tar.gz
Copy link
Contributor

Choose a reason for hiding this comment

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

why are you mirroring from your repo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For a stripped source tarball excluding examples, documentation and notebooks (4mb against >1Gb tarball)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nicogodet nicogodet marked this pull request as ready for review November 19, 2023 19:32
@nicogodet
Copy link
Contributor Author

i accept to be listed as maintainer.

ping @tomsail @brey

@nicogodet
Copy link
Contributor Author

It's ready for review @conda-forge/staged-recipes . Many thanks

@nicogodet nicogodet force-pushed the add-opentelemac branch 2 times, most recently from 40cfef7 to e7ae66d Compare November 20, 2023 08:11
@nicogodet
Copy link
Contributor Author

@conda-forge-admin, please ping conda-forge/staged-recipes. This PR is ready for review.

@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-webservice.

I was asked to ping @conda-forge/staged-recipes. and so here I am doing that.

Copy link

stale bot commented May 4, 2024

Hi friend!

We really, really, really appreciate that you have taken the time to make a PR on conda-forge/staged-recipes! conda-forge only exists because people like you donate their time to build and maintain conda recipes for use by the community.

In an effort to maintain this repository and increase the signal-to-noise for open PRs, the maintainers of staged-recipes close excessively old PRs after six months. This PR will remain open for another month, and then will be closed.

If you'd like to keep it open, please comment/push and we will be happy to oblige! Note that very old PRs will likely need to be rebased on main so that they can be rebuilt with the most recent CI scripts. If you have any trouble, or we missed reviewing this PR in the first place (sorry!), feel free to ping the team using a special command in a comment on the PR to get the attention of the staged-recipes team.

Cheers and thank you for contributing to this community effort!

@stale stale bot added the stale will be closed in 30 days label May 4, 2024
@tomsail
Copy link
Contributor

tomsail commented May 4, 2024

Hi @nicogodet, I'm keen to tackle this sometime in the following weeks. I'll ping you here when I catch up with the work you've done.

does windows release works in MPI without gotm? https://github.com/nicogodet/telemac-mascaret-feedstock

@stale stale bot removed the stale will be closed in 30 days label May 4, 2024
@nicogodet
Copy link
Contributor Author

@tomsail No... MPI is a PIA on windows

@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipes/opentelemac) and found some lint.

Here's what I've got...

For recipes/opentelemac:

  • The following maintainers have not yet confirmed that they are willing to be listed here: brey. Please ask them to comment on this PR if they are.

@tomsail
Copy link
Contributor

tomsail commented May 24, 2024

@brey please look at the above message.
If you're not keen, I'll remove you from the recipe maintainers.

@@ -0,0 +1,81 @@
{% set name = "opentelemac" %}
{% set version = "v8p4r1" %}
Copy link
Contributor

Choose a reason for hiding this comment

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

please update to v8p5r0 and upload the archive on your github repo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

While thinking back to it, it would be better to use archive from https://gitlab.pam-retd.fr/otm/telemac-mascaret

@brey
Copy link
Contributor

brey commented May 24, 2024

@brey please look at the above message. If you're not keen, I'll remove you from the recipe maintainers.

I think it's better if you remove me indeed. I am already in many places. Time to cut back

@@ -0,0 +1,32 @@
zip_keys:
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure you need this? Conda-forge defaults are not working for you here?

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks for the review felipe.
We have problems with certain versions of python/numpy (details there), hence the patch in recipes/opentelemac/patches.

I assume @nicogodet tried the conda-forge defaults

Copy link
Contributor

Choose a reason for hiding this comment

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

but I guess now we can remove this patch, since it has been integrated in main last year, correct?

@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/opentelemac) and found it was in an excellent condition.

@ocefpaf ocefpaf merged commit d24a8c3 into conda-forge:main May 26, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

5 participants