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 spyder-base package that does not depend on pyqt #203

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mrclary
Copy link
Contributor

@mrclary mrclary commented Nov 18, 2024

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.

Fixes #175
Fixes #178

This PR's workflows will fail until the changes in spyder-ide/spyder#23141 are propagated to PyPi.

spyder-ide/spyder#23141 can be merged anytime, since it should only impact installer builds for PRs. This PR should be merged some time after spyder-ide/spyder#23141 is merged, but immediately following upload of the PyPi package prior to a release or release candidate. Once this PR is merged, the remaining release steps can proceed.

Summary

This PR intends to create two packages from one feedstock.

  • spyder-base: contains the spyder source code and specifies all of spyder's dependencies except any direct or indirect dependence on pyqt.
  • spyder: only specifies spyder's direct and indirect dependencies on pyqt.

The intention is to provide a way for users to use pyside6 rather than pyqt, e.g.

conda install spyder-base pyside6

Installing spyder in the standard fashion will still yield a pyqt-based install.

conda install spyder

Notes

spyder-base was chosen as the "parent" package and spyder as the "subpackage" because the post-link scripts will only be included with the parent package and not the subpackage. Spyder's shortcut (via menuinst) is customized conditioned on the install environment, not target platform, and therefore can only use the post-link mechanism to achieve this. spyder-base must run the post-link scripts at install time, not spyder, in order to provide the shortcut in the case of using pyside6 rather than pyqt. If the Spyder shortcut did not require the post-link mechanism, then we could make spyder the parent package and spyder-base the subpackage.

python {{ python_min }} # [unix] was removed and other selectors were merged in the test environment(s) because it appears that they are not needed. The python spec is not needed for the tests to pass on macOS or Linux and cannot be present on Windows, else the tests fail (the test environment cannot be solved). Removing python and merging selectors in the spyder noarch package also helps ensure the build hash is the same across all platforms.

Concerns

I do not know which application Anaconda Navigator will provide: spyder-base, spyder, or both. The app section is not allowed in an item of the outputs section, i.e. the spyder package, so we cannot explicitly use it for the subpackage and remove it from the parent package. I suspect that Anaconda Navigator will recognize both packages as applications because the app header appears in the rendered meta.yaml for both packages, and this could be confusing. It may also be possible that Anaconda Navigator will only recognize the parent package as an application (i.e. spyder-base), which would be the worst outcome since this would not run without explicitly installing the remaining dependencies.

@mrclary
Copy link
Contributor Author

mrclary commented Nov 18, 2024

@conda-forge-admin, please rerender

@conda-forge-admin
Copy link
Contributor

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/meta.yaml) and found some lint.

Here's what I've got...

For recipe/meta.yaml:

  • ❌ The top level meta keys are in an unexpected order. Expecting ['package', 'source', 'build', 'requirements', 'test', 'app', 'outputs', 'about', 'extra'].
  • noarch packages can't have selectors. If the selectors are necessary, please remove noarch: python.

@mrclary
Copy link
Contributor Author

mrclary commented Nov 18, 2024

@conda-forge-admin, please rerender.

@conda-forge-admin
Copy link
Contributor

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/meta.yaml) and found it was in an excellent condition.

@conda-forge-admin
Copy link
Contributor

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

I tried to rerender for you, but it looks like there was nothing to do.

This message was generated by GitHub actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/11900049814.

@conda-forge-admin
Copy link
Contributor

conda-forge-admin commented Nov 21, 2024

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/meta.yaml) and found it was in an excellent condition.

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

For recipe/meta.yaml:

  • ℹ️ The recipe is not parsable by parser conda-souschef (grayskull). Your recipe may not receive automatic updates and/or may not be compatible with conda-forge's infrastructure. Please check the logs for more information and ensure your recipe can be parsed.

This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/11977420079. Examine the logs at this URL for more detail.

@mrclary
Copy link
Contributor Author

mrclary commented Nov 22, 2024

@conda-forge-admin , please rerender

recipe/meta.yaml Outdated Show resolved Hide resolved
recipe/meta.yaml Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@mrclary
Copy link
Contributor Author

mrclary commented Nov 22, 2024

@conda-forge-admin please rerender

@conda-forge-admin
Copy link
Contributor

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/meta.yaml) and found some lint.

Here's what I've got...

For recipe/meta.yaml:

  • ❌ The outputs section contained an unexpected subsection name. app is not a valid subsection name.

For recipe/meta.yaml:

  • ℹ️ The recipe is not parsable by parser conda-souschef (grayskull). Your recipe may not receive automatic updates and/or may not be compatible with conda-forge's infrastructure. Please check the logs for more information and ensure your recipe can be parsed.

This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/11980556058. Examine the logs at this URL for more detail.

@conda-forge-admin
Copy link
Contributor

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/meta.yaml) and found it was in an excellent condition.

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

For recipe/meta.yaml:

  • ℹ️ The recipe is not parsable by parser conda-souschef (grayskull). Your recipe may not receive automatic updates and/or may not be compatible with conda-forge's infrastructure. Please check the logs for more information and ensure your recipe can be parsed.

This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/11980594022. Examine the logs at this URL for more detail.

@mrclary
Copy link
Contributor Author

mrclary commented Nov 22, 2024

I tried to move the app section to outputs/app because I think we should want spyder to be the Anaconda Navigator app, not spyder-base, but it appears that the outputs/app section is not allowed.

For recipe/meta.yaml:

  • ❌ The outputs section contained an unexpected subsection name. app is not a valid subsection name.

@mrclary
Copy link
Contributor Author

mrclary commented Nov 22, 2024

For recipe/meta.yaml:

  • ℹ️ The recipe is not parsable by parser conda-souschef (grayskull). Your recipe may not receive automatic updates and/or may not be compatible with conda-forge's infrastructure. Please check the logs for more information and ensure your recipe can be parsed.

This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/11980556058. Examine the logs at this URL for more detail.

@hmaarrfk, do you have a recommendation to satisfy conda-souschef? It appears to be complaining about the duplicate key string, but these have selectors...

@mrclary mrclary marked this pull request as draft November 23, 2024 07:32
@mrclary mrclary marked this pull request as draft November 25, 2024 23:38
Copy link
Contributor

@hmaarrfk hmaarrfk left a comment

Choose a reason for hiding this comment

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

This looks great! i would squash all the changes and rerender in 2 commits to keep the history a little cleaner.

This is a "large" departure from previous efforts so it is good to keep things a little organized.

@mrclary
Copy link
Contributor Author

mrclary commented Dec 2, 2024

This looks great! i would squash all the changes and rerender in 2 commits to keep the history a little cleaner.

This is a "large" departure from previous efforts so it is good to keep things a little organized.

Thanks @hmaarrfk, I'll do that.

Do you have any guidance on the concerns I listed in the PR description?

@hmaarrfk
Copy link
Contributor

hmaarrfk commented Dec 2, 2024

I do not know which application Anaconda Navigator will provide: spyder-base, spyder, or both.

This is generally a "Anaconda" problem. I think that for now, spyder + qtpy5 is still the recommended installation process so hopefully spyder remains there as the program but I don't know....

The spyder subpackage is completely platform independent and noarch, since it has no source code.

Thats ok, only the "first" one will get uploaded the others will not get uploaded. so long as you don't "pin exact" it should be fine

On rerender, since spyder-base is the parent package, the README.md now contains references to spyder-base-feedstock, which does not exist. I do not know how to fix this, nor do I know how other feedstocks have avoided this (e.g. qtconsole-feedstock)

add:

  feedstock-name: spyder

@mrclary mrclary force-pushed the spyder-base branch 4 times, most recently from da63769 to d87c91f Compare December 2, 2024 19:48
@hmaarrfk
Copy link
Contributor

hmaarrfk commented Dec 2, 2024

Thanks for working through all the challenges. Splitting a package is definitely challenging

@mrclary
Copy link
Contributor Author

mrclary commented Dec 2, 2024

Thanks for working through all the challenges. Splitting a package is definitely challenging

Thank you for your help reviewing and addressing my concerns. 👏🏼

@mrclary mrclary marked this pull request as ready for review December 6, 2024 07:36
@mrclary mrclary removed request for andfoy and steff456 December 6, 2024 07:36
@mrclary mrclary force-pushed the spyder-base branch 2 times, most recently from bca9e10 to a2a9e57 Compare December 6, 2024 19:07
@mrclary mrclary marked this pull request as draft December 7, 2024 15:44
@mrclary mrclary force-pushed the spyder-base branch 2 times, most recently from f6bbfaf to 6cc32fd Compare December 10, 2024 04:27
Use jinja variable for build number and bump.
Remove reference to python version in tests to ensure that spyder package has same hash regardless of runner.
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.

ENH: Add a way to install Spyder with pyqt6 or pyside6 Making webengine optional on conda-forge
3 participants