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 constructor manager api #46

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
66 changes: 41 additions & 25 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,16 @@ on:
- main
paths:
- 'constructor-manager/**'
- 'constructor-manager-cli/**'
- 'constructor-manager-api/**'
workflow_dispatch:

jobs:
test:
name: ${{ matrix.platform }} py${{ matrix.python-version }}
runs-on: ${{ matrix.platform }}
defaults:
run:
shell: bash -el {0}
strategy:
matrix:
platform: [ubuntu-latest, windows-latest, macos-latest]
Expand All @@ -25,36 +28,49 @@ jobs:
- uses: actions/checkout@v3

- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v4
uses: conda-incubator/setup-miniconda@v2
with:
python-version: ${{ matrix.python-version }}
activate-environment: ""
auto-activate-base: true

- name: Install dependencies constructor-manager-cli
- name: Install dependencies on main environment
run: |
python -m pip install --upgrade pip
python -m pip install setuptools tox tox-gh-actions
cd constructor-manager-cli
pip install -e .
pip list
conda install -n base conda-lock mamba -y -c conda-forge
conda list

- name: Test constructor-manager-cli
- name: create constructor-manager-environment
run: |
cd constructor-manager-cli
python -m tox
env:
PLATFORM: ${{ matrix.platform }}
conda create -n constructor-manager conda packaging requests pyyaml -y -c conda-forge
conda activate constructor-manager
# List installed packages
conda list

- name: Install dependencies constructor-manager
- name: Install constructor-manager
run: |
cd constructor-manager-cli
pip install -e .
pip list
env:
PLATFORM: ${{ matrix.platform }}
conda activate constructor-manager
# Install constructor manager
git clone https://github.com/goanpeca/packaging.git packaging_clone
cd packaging_clone
git checkout constructor-updater
cd constructor-manager
pip install -e . --no-deps
# Install test deps
conda install -n constructor-manager pytest pytest-cov pytest-qt -c conda-forge -y
# List installed packages
conda list

- name: Test constructor-manager
- name: Install constructor-manager-api
run: |
cd constructor-manager
python -m tox
env:
PLATFORM: ${{ matrix.platform }}
conda activate constructor-manager
conda install -n constructor-manager qtpy pyqt -y -c conda-forge
cd constructor-manager-api
pip install -e . --no-deps

- name: Test constructor-manager-api
run: |
conda activate constructor-manager
# List installed packages
conda list
cd constructor-manager-api/src
# Run Tests
pytest constructor_manager_api --cov=constructor_manager_api
File renamed without changes.
File renamed without changes.
25 changes: 25 additions & 0 deletions constructor-manager-api/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# Constructor manager API

## Requirements

- qtpy
- constructor-manager-cli (on base environment)

## Usage

```python
from constructor_manager.api import check_updates
Copy link
Member

Choose a reason for hiding this comment

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

With the directory structure change I think this should be something like:

Suggested change
from constructor_manager.api import check_updates
from constructor_manager_api.api import check_updates

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, but I think I prefer adding those imports imports on the init so there is no need for the second api submodule (which looks weird :)



def finished(result):
print(result)


worker = check_updates(package_name="napari", current_version="0.4.10", channel="conda-forge")
Copy link
Member

@dalthviz dalthviz Mar 13, 2023

Choose a reason for hiding this comment

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

Checking the check_updates function signature I think this should be something like:

Suggested change
worker = check_updates(package_name="napari", current_version="0.4.10", channel="conda-forge")
worker = check_updates(package_name="napari", current_version="0.4.10", channels=["conda-forge"])

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

worker.finished.connect(finished)
worker.start()
```

## License

Distributed under the terms of the MIT license. is free and open source software
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
[metadata]
name = constructor-manager
version = 0.0.1
description = Constructor environment and updates manager API
name = constructor-manager-api
version = 0.1.0
description = TODO
long_description = file: README.md
long_description_content_type = text/markdown
url = https://github.com/napari/packaging/constructor-manager
author = napari team
author_email = [email protected]
url = https://github.com/napari/packaging/constructor-manager-api
author = napari
author_email = TODO
Copy link
Member

Choose a reason for hiding this comment

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

I guess there is not a email to use here, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not sure what email we should use 🤔

@kephale do you have a suggestion ?

Copy link

Choose a reason for hiding this comment

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

[email protected] is still the official napari SC email address, this can be reverted.

license = MIT
license_files = LICENSE
classifiers =
Expand All @@ -23,7 +23,7 @@ classifiers =
Topic :: Scientific/Engineering :: Image Processing
project_urls =
Bug Tracker = https://github.com/napari/packaging/issues
Source Code = https://github.com/napari/packaging/constructor-manager
Source Code = https://github.com/napari/packaging/constructor-manager-api
Copy link
Member

Choose a reason for hiding this comment

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

Should pyyaml be added in the install_requires section of this package? I tried doing:

from constructor_manager_api.api import check_updates

and I got an error related to the import yaml done for the settings module:

>>> from constructor_manager_api.api import check_updates
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "E:\Acer\Documentos\Quansight\Napari\packaging-otros\gonzalo\packaging\constructor-manager-api\src\constructor_manager_api\api.py", line 10, in <module>
    from constructor_manager_api.utils.settings import save_settings
  File "E:\Acer\Documentos\Quansight\Napari\packaging-otros\gonzalo\packaging\constructor-manager-api\src\constructor_manager_api\utils\settings.py", line 5, in <module>
    import yaml
ModuleNotFoundError: No module named 'yaml'

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes you are right!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed


[options]
packages = find:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
VERSION_INFO = (0, 1, 0)
__version__ = "0.1.0"
Loading