update framework #77

casperdcl · 2023-07-06T10:31:00Z

Part of #76, closes #11, closes #54, fixes #56

migrate setup.py => pyproject.toml
- use SCM versioning
- drop unused sphinx dependency
migrate unittest => pytest
- fix & update some tests
add CI tests & update workflows
- purge conda packaging (will do in upstream conda-forge repo instead)
misc code tidy
- esp. reduce duplication in test/test__formUI_status_test.py
add qdarkstyle dependency
misc minor metadata updates
- minify licen[cs]e
- purge changelog in favour of https://github.com/paskino/qt-elements/releases
enforce annotated tags for releases
https://github.com/paskino/qt-elements/transfer to @TomographicImaging
- update URLs

follow-up issues for separate PRs

see https://github.com/paskino/qt-elements/releases instead

examples/reorderable_list_widget_example.py

test/test_MainWindowWithSessionManagement.py

lauramurgatroyd

Thanks, it looks great! Please see my comments 😄
Nice work on tidying the tests!

lauramurgatroyd · 2023-07-06T14:53:46Z

CHANGELOG.md

@@ -1,66 +0,0 @@
-


I think we need to keep the CHANGELOG.
The way we use it is, when anything is merged to master, we add a description of the changes to the change log, for version x.x.x, this continues until we decide to release. Then when we release we copy over the contents of the change log.
So we use it as we go along, before releases.

Committed CHANGELOG.txt files were indeed standard in the days of mercurial & subversion, but are becoming less common. A few reasons I can think of:

adhering to SSoT/DRY principles, i.e. no duplication across CHANGELOG.txt and github.com/OWNER/REPO/releases

btw they were out of sync in this repo - I had to update some of the release notes & add a few missing releases in https://github.com/paskino/qt-elements/releases

not bloating repo size & reducing download time

triggering CI

GHA can run workflows on.release

it's far less common & more complex/error-prone to use on.push.paths

webscrapers, automated parsing, webhooks & external tools

e.g. zenodo.org picks up changelogs from GH releases' notes rather than changelog files

Meanwhile, for progressively building pre-release notes, you could use one of the following places:

pull request descriptions

commit messages

draft (unpublished) releases

Note that (for the lazy) the latter 2 options are combined as per this¹, though personally I find it best to put effort into the first option².

Always happy to discuss alternatives though :)

Footnotes

which is a lightweight version of GH's IMO-too-verbose-eyecandy autogenerated notes ↩

e.g. https://github.com/NiftyPET/NIMPA/pull/28#issue-1732872191 -> https://github.com/NiftyPET/NIMPA/releases/tag/v2.6.0 ↩

lauramurgatroyd · 2023-07-06T14:57:15Z

LICENSE

+Copyright 2020 Edoardo Pasca

-   TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at

-   1. Definitions.
+    http://www.apache.org/licenses/LICENSE-2.0


We need to discuss with @paskino about the licensing, headers, etc. but all contributors should be listed and it's copyright to the institution:

Copyright 2020 United Kingdom Research and Innovation

Then the authors list is separate

Authors:
Edoardo Pasca
etc.

Updated.
Also worth a follow-up issue/discussion I think.

eqt/io.py

lauramurgatroyd · 2023-07-06T15:13:28Z

conda/meta.yaml

@@ -1,36 +0,0 @@
-package:
-  name: eqt


My understanding is that the conda build will happen somehow with conda-forge but will we not need any of these files for it?

no, it won't be needed :)

lauramurgatroyd · 2023-07-06T15:52:20Z

pyproject.toml

+[project.urls]
+documentation = "https://github.com/paskino/qt-elements#readme"
+repository = "https://github.com/paskino/qt-elements"
+changelog = "https://github.com/paskino/qt-elements/releases"


When/if we reinstate the changelog.md, we can change the link here

.github/workflows/test.yml

lauramurgatroyd · 2023-07-06T16:24:42Z

.github/workflows/test.yml

+        changelog=$(git log --pretty='format:%d%n- %s%n%b---' $(git tag --sort=v:refname | tail -n2 | head -n1)..HEAD)
+        tag="${GITHUB_REF#refs/tags/}"
+        gh release create --title "Version $tag" --draft --notes "$changelog" "$tag" dist/${{ steps.dist.outputs.whl }} dist/${{ steps.dist.outputs.targz }}


Does this automatically create the version description from the git logs?
I think we would prefer to use the change log we manually write in CHANGELOG.md

yes but as per the updated .github/workflows/README.md instructions; it only creates a --draft release. Manual copyediting & approval of said draft is always required to make it public.

So the concept here is that we use git log to create the draft of the release text, taking from the commit messages. This text accompanies a GitHub draft release and will then be manually edited once we make the actual release.

I like the automation, however I feel that the release text is related to a GitHub feature rather than an integral part of the software.

Often I find myself editing the changelog differently than the commit messages. Maybe it is a good practice to write the commit messages better! My real argument against deleting the CHANGELOG.md is that we tend to cluster changes on topic rather than commit.

As I understand it, there will still be the chance to edit the draft text, but how can we find the final text not on GitHub?

I like the automation

It's not about automation. Automation is merely a happy bonus side-effect. It's about DRY/SSoT (see full explanation, including bugs I had to manually fix in the old release noted).

I feel that the release text is related to a GitHub feature rather than an integral part of the software.

the GH release text was already being used before this PR

dev metadata (version history) is NOT meant to be an "integral part of software" (that's the whole point of git commits as well as commit messages)

best practice is to put release notes as comments in annotated tags or merge commits if you want them in a place where a dev will find them easily sans GH web interface

release notes should link to relevant issues & PRs (for context & discussion), which is distinctly inherently a GH-only (non-local-codebase) use case.

Often I find myself editing the changelog differently than the commit messages. Maybe it is a good practice to write the commit messages better!

Still possible here. I do the same. The commit messages injected in the draft ensure:

I don't forget to mention something important

I'm embarrassed by poorly-written unfit-for-purpose messages (which I delete), inspiring me to write better in future

My real argument against deleting the CHANGELOG.md is that we tend to cluster changes on topic rather than commit.

that's what merge commits & tags are for, ergo what should be described in merge commit messages & annotated tags

the changelog was mirrored in two places, I'm simply proposing we delete one of those places

As I understand it, there will still be the chance to edit the draft text, but how can we find the final text not on GitHub?

Annotated tags & (merge) commit messages.

Finally, overall harsh truth:

high-level end users: only look at web material (PyPI, GH Readme, GH releases)

devs: only look at git commits & git blame

nobody: wants changelog file checked out in-repo

I really believe there is zero frustration if changelog.txt simply links to GH releases.

@paskino as discussed, I've now updated all git tags to have annotations containing the release notes.

You can git fetch --tags -f to get the updated annotations. To view a changelog without an internet connection¹:

git config --global alias.changelog 'for-each-ref --sort=-*authordate --format="# %(contents:subject)%0a%(contents:body)" refs/tags' git changelog

next step is to https://github.com/paskino/qt-elements/transfer to @TomographicImaging

Footnotes

https://stackoverflow.com/questions/69765351/list-all-git-tags-with-their-annotations ↩

added instructions & tests for annotated tags: 4e918e6

test/dialog_example_2_test.py

test/test_MainWindowWithSessionManagement.py

pyproject.toml

paskino

It looks great. Please keep the changelog as it is a file which belongs to the repository.

I'd like to understand how to use pytest in the future.

paskino · 2023-07-07T08:21:05Z

.github/workflows/test.yml

+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        python: [3.7, 3.11]


will the binaries built be only available for Python 3.7 and 3.11? In such a case, I'd like to be sure that these are available for all Python versions compatible with CIL, which are from 3.8 to 3.10!

No, binaries are built for all python versions. CI unit tests are only run for min & max supported versions.

paskino · 2023-07-07T08:34:06Z

.github/workflows/test.yml

+        changelog=$(git log --pretty='format:%d%n- %s%n%b---' $(git tag --sort=v:refname | tail -n2 | head -n1)..HEAD)
+        tag="${GITHUB_REF#refs/tags/}"
+        gh release create --title "Version $tag" --draft --notes "$changelog" "$tag" dist/${{ steps.dist.outputs.whl }} dist/${{ steps.dist.outputs.targz }}


So the concept here is that we use git log to create the draft of the release text, taking from the commit messages. This text accompanies a GitHub draft release and will then be manually edited once we make the actual release.

I like the automation, however I feel that the release text is related to a GitHub feature rather than an integral part of the software.

Often I find myself editing the changelog differently than the commit messages. Maybe it is a good practice to write the commit messages better! My real argument against deleting the CHANGELOG.md is that we tend to cluster changes on topic rather than commit.

As I understand it, there will still be the chance to edit the draft text, but how can we find the final text not on GitHub?

paskino · 2023-07-09T21:06:03Z

test/dialog_example_2_test.py

@@ -82,8 +72,7 @@ def rejected(self):

 class DialogTest(unittest.TestCase):
    '''Test the margarita mixer GUI'''
-
-    @unittest.skipIf(skip_as_conda_build, "On conda builds do not do any test with interfaces")
+    @skip_ci


How does a decorator work?

It was a decorator before too - I just tidied it up so the condition & message aren't repeated a hundred times in the code.

README.md

casperdcl added 8 commits July 5, 2023 13:01

purge changelog

28a176b

see https://github.com/paskino/qt-elements/releases instead

minify licen[cs]e

13ce71f

migrate setup.py => pyproject.toml

a57a443

ci: update PyPI workflow

7e5d3bc

deps: add qdarkstyle

0a356eb

build: purge conda packaging

df36485

tests: migrate to pytest

28ee6bc

tests: add CI config

107cbd7

casperdcl added enhancement New feature or request tests gh-actions labels Jul 6, 2023

casperdcl self-assigned this Jul 6, 2023

build: fix readme metadata

339f16d

casperdcl commented Jul 6, 2023

View reviewed changes

examples/reorderable_list_widget_example.py Show resolved Hide resolved

casperdcl requested review from paskino and lauramurgatroyd July 6, 2023 10:38

casperdcl mentioned this pull request Jul 5, 2023

release on conda-forge #76

Closed

8 tasks

casperdcl added 2 commits July 6, 2023 15:15

minor tidy

c321b8c

tests: DRY

29cf08d

casperdcl force-pushed the update-framework branch from 4224fa5 to 29cf08d Compare July 6, 2023 14:38

casperdcl commented Jul 6, 2023

View reviewed changes

test/test_MainWindowWithSessionManagement.py Show resolved Hide resolved

lauramurgatroyd reviewed Jul 6, 2023

View reviewed changes

review comments

4ba9d8e

paskino requested changes Jul 12, 2023

View reviewed changes

build: enforce annotated tags

4e918e6

casperdcl force-pushed the update-framework branch from 70c2eff to 4e918e6 Compare July 17, 2023 18:05

build: skip PyPI upload on existing version

756ea95

paskino reviewed Jul 19, 2023

View reviewed changes

README.md Outdated Show resolved Hide resolved

update URLs after repo migration

f3ca0dd

casperdcl merged commit ade2a2d into main Jul 19, 2023
5 checks passed

casperdcl deleted the update-framework branch July 19, 2023 16:52

casperdcl mentioned this pull request Jul 19, 2023

fix failing test #78

Closed

casperdcl mentioned this pull request Aug 7, 2023

Can we delete the conda_build_config file? #56

Closed

paskino mentioned this pull request Sep 13, 2023

paskino conda channel not required TomographicImaging/CILViewer#366

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update framework #77

update framework #77

casperdcl commented Jul 6, 2023 •

edited

Loading

lauramurgatroyd left a comment

lauramurgatroyd Jul 6, 2023

casperdcl Jul 6, 2023 •

edited

Loading

lauramurgatroyd Jul 6, 2023 •

edited

Loading

casperdcl Jul 6, 2023

lauramurgatroyd Jul 6, 2023

casperdcl Jul 6, 2023

lauramurgatroyd Jul 6, 2023

lauramurgatroyd Jul 6, 2023

casperdcl Jul 6, 2023

paskino Jul 7, 2023

casperdcl Jul 13, 2023 •

edited

Loading

casperdcl Jul 17, 2023 •

edited

Loading

casperdcl Jul 17, 2023 •

edited

Loading

paskino left a comment

paskino Jul 7, 2023

casperdcl Jul 13, 2023

paskino Jul 7, 2023

paskino Jul 9, 2023

casperdcl Jul 13, 2023

update framework #77

update framework #77

Conversation

casperdcl commented Jul 6, 2023 • edited Loading

follow-up issues for separate PRs

lauramurgatroyd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

casperdcl Jul 6, 2023 • edited Loading

Choose a reason for hiding this comment

Footnotes

lauramurgatroyd Jul 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

casperdcl Jul 13, 2023 • edited Loading

Choose a reason for hiding this comment

casperdcl Jul 17, 2023 • edited Loading

Choose a reason for hiding this comment

Footnotes

casperdcl Jul 17, 2023 • edited Loading

Choose a reason for hiding this comment

paskino left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

casperdcl commented Jul 6, 2023 •

edited

Loading

casperdcl Jul 6, 2023 •

edited

Loading

lauramurgatroyd Jul 6, 2023 •

edited

Loading

casperdcl Jul 13, 2023 •

edited

Loading

casperdcl Jul 17, 2023 •

edited

Loading

casperdcl Jul 17, 2023 •

edited

Loading