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

python311Packages.{shiny, rsconnect_*} #231189

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

nviets
Copy link
Contributor

@nviets nviets commented May 10, 2023

Description of changes

Adding Shiny for Python, Posit Connect packages, and related dependencies.

@alexvorobiev @onny

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested, as applicable:

@nviets
Copy link
Contributor Author

nviets commented May 10, 2023

@onny - how's this? Thanks for the feedback so far.

@ofborg ofborg bot added 2.status: merge conflict This PR has merge conflicts with the target branch 8.has: package (new) This PR adds a new package 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 and removed 2.status: merge conflict This PR has merge conflicts with the target branch labels May 12, 2023
@nviets
Copy link
Contributor Author

nviets commented May 15, 2023

Result of nixpkgs-review pr 231189 run on x86_64-linux 1

20 packages built:
  • python310Packages.contextvars
  • python310Packages.contextvars.dist
  • python310Packages.htmltools
  • python310Packages.htmltools.dist
  • python310Packages.rsconnect-jupyter
  • python310Packages.rsconnect-jupyter.dist
  • python310Packages.rsconnect-python
  • python310Packages.rsconnect-python.dist
  • python310Packages.shiny
  • python310Packages.shiny.dist
  • python310Packages.shinywidgets
  • python310Packages.shinywidgets.dist
  • python311Packages.contextvars
  • python311Packages.contextvars.dist
  • python311Packages.htmltools
  • python311Packages.htmltools.dist
  • python311Packages.rsconnect-jupyter
  • python311Packages.rsconnect-jupyter.dist
  • python311Packages.rsconnect-python
  • python311Packages.rsconnect-python.dist

@nviets
Copy link
Contributor Author

nviets commented Jul 1, 2023

Result of nixpkgs-review pr 231189 run on x86_64-linux 1

20 packages built:
  • python310Packages.contextvars
  • python310Packages.contextvars.dist
  • python310Packages.htmltools
  • python310Packages.htmltools.dist
  • python310Packages.rsconnect-jupyter
  • python310Packages.rsconnect-jupyter.dist
  • python310Packages.rsconnect-python
  • python310Packages.rsconnect-python.dist
  • python310Packages.shiny
  • python310Packages.shiny.dist
  • python310Packages.shinywidgets
  • python310Packages.shinywidgets.dist
  • python311Packages.contextvars
  • python311Packages.contextvars.dist
  • python311Packages.htmltools
  • python311Packages.htmltools.dist
  • python311Packages.rsconnect-jupyter
  • python311Packages.rsconnect-jupyter.dist
  • python311Packages.rsconnect-python
  • python311Packages.rsconnect-python.dist

@nviets
Copy link
Contributor Author

nviets commented Jul 1, 2023

Any feedback is welcome, otherwise this is ready for a merge. Thanks!

@nviets
Copy link
Contributor Author

nviets commented Jul 1, 2023

@onny - thanks!

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-in-distress/3604/87

@pbsds pbsds changed the title python310Packages.{shiny, rsconnect_*} python311Packages.{shiny, rsconnect_*} Feb 20, 2024
Copy link
Member

@pbsds pbsds left a comment

Choose a reason for hiding this comment

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

Result of nixpkgs-review pr 231189 --eval local run on x86_64-linux 1

2 packages failed to build:
  • python311Packages.rsconnect-jupyter
  • python311Packages.rsconnect-jupyter.dist
12 packages built:
  • python311Packages.contextvars
  • python311Packages.contextvars.dist
  • python311Packages.htmltools
  • python311Packages.htmltools.dist
  • python311Packages.rsconnect-python
  • python311Packages.rsconnect-python.dist
  • python311Packages.shiny
  • python311Packages.shiny.dist
  • python311Packages.shinywidgets
  • python311Packages.shinywidgets.dist
  • python312Packages.contextvars
  • python312Packages.contextvars.dist
rsconnect-jupyter> E   ModuleNotFoundError: No module named 'notebook.base'

There are some systemic issues i only comment on once, please apply the changes to all the added packages.

Please also clean up the git commit history, notably:

  • contextvars and shiny are added in the same commit
  • the final commit silently bumps two deps

Comment on lines +34 to +42
nativeCheckInputs = [
pytest
];

checkPhase = ''
runHook preCheck
pytest tests/
runHook postCheck
'';
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
nativeCheckInputs = [
pytest
];
checkPhase = ''
runHook preCheck
pytest tests/
runHook postCheck
'';
nativeCheckInputs = [
pytestCheckHook
];

buildPythonPackage rec {
pname = "contextvars";
version = "2.4";
format = "setuptools";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
format = "setuptools";
pyproject = true;

A new standard

# can reenable once upstream updates
checkPhase = ''
runHook preCheck
pytest tests/ --ignore=tests/test_managers.py
Copy link
Member

Choose a reason for hiding this comment

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

When using pytestCheckHook you can add additional flags to pytestFlagsArray, but this case can be solved with disabledTests or disabledTestPaths

checkPhase = ''
runHook preCheck
# Needed to avoid /homeless-shelter error
export HOME=$(mktemp -d)
Copy link
Member

Choose a reason for hiding this comment

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

when using pytestCheckHook, this can be moved to preCheck

@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 19, 2024
@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label May 22, 2024
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label May 22, 2024
@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 4, 2024
@aucub aucub closed this Dec 19, 2024
@pbsds
Copy link
Member

pbsds commented Dec 21, 2024

@aucub, why was this closed?

@aucub
Copy link
Contributor

aucub commented Dec 21, 2024

Some packages have been added

@aucub aucub reopened this Dec 21, 2024
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Dec 21, 2024
@nix-owners nix-owners bot requested a review from natsukium December 21, 2024 00:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 6.topic: python 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants