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

Fixing r41 issue #12

Merged
merged 9 commits into from
Nov 28, 2023
Merged

Fixing r41 issue #12

merged 9 commits into from
Nov 28, 2023

Conversation

cofinoa
Copy link
Contributor

@cofinoa cofinoa commented Nov 24, 2023

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

@cofinoa cofinoa requested a review from a team as a code owner November 24, 2023 18:00
@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 (recipe) and found it was in an excellent condition.

@cofinoa
Copy link
Contributor Author

cofinoa commented Nov 24, 2023

@conda-forge-admin, please rerender

conda-forge-webservices[bot] and others added 2 commits November 24, 2023 18:02
@cofinoa
Copy link
Contributor Author

cofinoa commented Nov 24, 2023

@conda-forge-admin, please rerender

conda-forge-webservices[bot] and others added 2 commits November 24, 2023 19:02
@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 (recipe) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipe:

@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 (recipe) and found some lint.

Here's what I've got...

For recipe:

  • Failed to even lint the recipe, probably because of a conda-smithy bug 😢. This likely indicates a problem in your meta.yaml, though. To get a traceback to help figure out what's going on, install conda-smithy and run conda smithy recipe-lint . from the recipe directory.

@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 (recipe) and found it was in an excellent condition.

Copy link
Member

@mfansler mfansler left a comment

Choose a reason for hiding this comment

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

The issue is not that a jpeg dependency is needed here - noarch R packages never need shared libraries in host. Rather r-jpeg exclusively used jpeg to supply libjpeg for all of its linux-64 R 4.1 builds on Conda Forge. When Conda Forge changed to libjpeg-turbo to supply libjpeg, it was only building R 4.2 and 4.3 for linux-64. Hence, there are no linux-64 R 4.1 builds of r-jpeg that work with the libjpeg-turbo library.

The error encountered is because some of the old r-jpeg builds didn't properly declare their dependencies in the metadata. See conda-forge/r-jpeg-feedstock#13.

The solution is to get a newer linux-64 R 4.1 r-jpeg build made.

However, the motivation here is not explicated. It has been pointed out previously that providing extended older R version support is not generally sustainable. Conda Forge currently builds for R 4.2 and 4.3, which some partial support for Windows where we can only build R 4.1. Outside of these, it is important to communicate why there is a need to use Conda Forge resources to generate out-of-cycle builds.

@cofinoa
Copy link
Contributor Author

cofinoa commented Nov 25, 2023

The motivation for extended support of R older versions is to provide reproducibility in R3.6, which is the older version supported. Migration to R>=4.0 it's on going but support for R3.6 it's still requested, and no decision has been yet made on which version of R>=4.0 will be become the reference for future releases.

This package (r-visualizer) it's part of the climate4R framework with many other packages and interdependencies should be taken in consideration. That means, if this package (r-visualizer) which is a core package it's not supported in R4.1 all its dependants won't.

I have just made a patch to support R4.1, but just to be considering that R4.1 it's been supported for the rest of packages.

From your comment, it's clear that for future releases/builds, R4.1 should be considered to be removed.

@cofinoa cofinoa merged commit 5006cc4 into conda-forge:main Nov 28, 2023
7 checks passed
@cofinoa cofinoa deleted the fixing-r41-issue branch November 28, 2023 17:56
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