Prepare release of version 1.0.0 #64

sdjmchattie · 2024-02-14T16:13:05Z

Changes proposed in this pull request

Add in the changes to use Traction v2 API data format.

Instructions for Reviewers

Bumps tj-actions/changed-files from 23 to 41.

Bumps [requests](https://github.com/psf/requests) from 2.28.2 to 2.31.0. - [Release notes](https://github.com/psf/requests/releases) - [Changelog](https://github.com/psf/requests/blob/main/HISTORY.md) - [Commits](psf/requests@v2.28.2...v2.31.0) --- updated-dependencies: - dependency-name: requests dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]>

Updated readme to include snappy fix.

Bump requests from 2.28.2 to 2.31.0

Bumps [certifi](https://github.com/certifi/python-certifi) from 2022.12.7 to 2023.7.22. - [Commits](certifi/python-certifi@2022.12.07...2023.07.22) --- updated-dependencies: - dependency-name: certifi dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]>

Bumps [urllib3](https://github.com/urllib3/urllib3) from 1.26.14 to 1.26.18. - [Release notes](https://github.com/urllib3/urllib3/releases) - [Changelog](https://github.com/urllib3/urllib3/blob/main/CHANGES.rst) - [Commits](urllib3/urllib3@1.26.14...1.26.18) --- updated-dependencies: - dependency-name: urllib3 dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]>

Bumps [aiohttp](https://github.com/aio-libs/aiohttp) from 3.8.3 to 3.9.0. - [Release notes](https://github.com/aio-libs/aiohttp/releases) - [Changelog](https://github.com/aio-libs/aiohttp/blob/master/CHANGES.rst) - [Commits](aio-libs/aiohttp@v3.8.3...v3.9.0) --- updated-dependencies: - dependency-name: aiohttp dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]>

Bump certifi from 2022.12.7 to 2023.7.22

Bump urllib3 from 1.26.14 to 1.26.18

Bump aiohttp from 3.8.3 to 3.9.0

Bumps [aiohttp](https://github.com/aio-libs/aiohttp) from 3.9.0 to 3.9.2. - [Release notes](https://github.com/aio-libs/aiohttp/releases) - [Changelog](https://github.com/aio-libs/aiohttp/blob/master/CHANGES.rst) - [Commits](aio-libs/aiohttp@v3.9.0...v3.9.2) --- updated-dependencies: - dependency-name: aiohttp dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]>

Bump aiohttp from 3.9.0 to 3.9.2

Also lay the docstring out according to Google's style guide

DPL-1055: Sample names in QC results

It doesn't make sense for it to be a dictionary when we iterate over it by expecting all keys in a range the same size as the length of the entries in the dictionary!

This also includes those for OutputTractionMessageInterface and TractionQcMessageInterface

Code elsewhere assumes the indices are non-sparse so we should be using a list

DPL-952: Refactor the code before applying the update for Traction V2

DPL-952: Support traction V2 API

yoldas · 2024-02-15T16:03:18Z

tests/messages/test_traction_qc_message.py

+        request.post_spri_volume = "20"
+        request.final_nano_drop_280 = "280"
+        request.final_nano_drop_230 = "230"
+        request.final_nano_drop = "200"


Could you confirm that representing numbers as strings is correct use?

Yes they are strings. You can see that here: https://github.com/sanger/tol-lab-share/blob/develop/tol_lab_share/messages/traction_qc_message.py#L24-L33

yoldas · 2024-02-15T16:05:05Z

tests/messages/test_traction_qc_message.py

@@ -79,7 +83,7 @@ def test_can_generate_correct_payload(self, valid_traction_qc_message):
                            "shearing_qc_comments": "Comments",
                            "date_submitted": datetime.now().timestamp() * 1000,


datetime.now without timezone is locale dependent I suppose. Does this matter?

This is fine in a test. I would be concerned if it was part of the code creating messages or logs to be sent out. I'll update it in the test anyway to avoid misleading someone who sees this and thinks that's how you create the timestamp in messages.

yoldas · 2024-02-15T16:07:40Z

tol_lab_share/config/logging.py


 DEFAULT_LOGGING_LEVEL = "INFO"

-LOGGING: Dict[str, Any] = {
+LOGGING: dict[str, Any] = {


I was assuming type declaration is done uppercase but lowercase seems fine. As the Dict is removed from import statement from typing module, could you confirm using lowercase dict here is right?

Since Python 3.9, they've removed the redundancy of having List and Dict in the typing package for the use of generics. In the past, if you used the lowercase list and dict you couldn't specify the inner types of those. That was worked around using the typing package, but that's no longer necessary due to improvements to the core Python language, so I got rid of them.

https://peps.python.org/pep-0585/

yoldas · 2024-02-15T16:13:01Z

tol_lab_share/message_properties/definitions/labware_type.py

@@ -44,24 +44,24 @@ def pad_number(self, number: int) -> str:
            padded_number = f"0{padded_number}"


if padded_number is 10, this will make 010 . Is this correct?

No because the line before it checks for a length 1 string, but I don't know why Eduardo implemented it this way when str has a zfill() method precisely for padding numbers with zeroes. I'll update it.

yoldas

I have added a few questions to make sure. Looks good.

Remove the padding method and turn the location creation into a list comprehension

Not strictly necessary, but it's better practice than the locale affected now() method

DPL-952: Remaining Fixes

yoldas and others added 30 commits January 3, 2024 00:31

bump tj-actions/changed-files from 23 to 41 in /.github/workflows

4b1d7d2

Bumps tj-actions/changed-files from 23 to 41.

Updated readme to include snappy fix.

ec56b63

Merge pull request #51 from sanger/update-readme-to-include-snappy-fix

ebc0c55

Updated readme to include snappy fix.

Merge pull request #50 from sanger/dependabot/pip/requests-2.31.0

527aeda

Bump requests from 2.28.2 to 2.31.0

Merge pull request #52 from sanger/dependabot/pip/certifi-2023.7.22

7bd154b

Bump certifi from 2022.12.7 to 2023.7.22

Merge pull request #53 from sanger/dependabot/pip/urllib3-1.26.18

4671937

Bump urllib3 from 1.26.14 to 1.26.18

Merge pull request #54 from sanger/dependabot/pip/aiohttp-3.9.0

900b063

Bump aiohttp from 3.8.3 to 3.9.0

Update instructions around snappy installation

44020c6

Fix Markdown formatting

fc8bb18

Improve documentation markdown files

ef83566

Merge pull request #56 from sanger/dependabot/pip/aiohttp-3.9.2

1747d21

Bump aiohttp from 3.9.0 to 3.9.2

Fix typo in docstring

674c6f7

Also lay the docstring out according to Google's style guide

Switch Traction payload to receive Sanger Sample ID

83592ef

Accept black reformat

31068c8

Merge pull request #58 from sanger/dpl-1055-sample-names-in-qc-results

eb29fb2

DPL-1055: Sample names in QC results

Do some tidy up of docstrings

412108d

Change requests to a list

cb58c41

It doesn't make sense for it to be a dictionary when we iterate over it by expecting all keys in a range the same size as the length of the entries in the dictionary!

Remove interfaces around MessageProperty

8897f9e

This also includes those for OutputTractionMessageInterface and TractionQcMessageInterface

Remove remaining unneeded interfaces

cd0bb8d

Remove unnecessary property boilerplate on MessageProperty

0cd5e69

Remove boilerplate for instance variables in all message types

a989b60

Change Dict and List imports to built in types

1d702b4

Remove remaining Optionals

55ce334

Remove Unions

be40f38

Change TractionQcMessage requests to a list

f6d7d29

Code elsewhere assumes the indices are non-sparse so we should be using a list

sdjmchattie and others added 13 commits February 12, 2024 17:15

Update expected payload in unit tests for V2 Traction API

1d50f78

Update the serializer for tube requests

0b9fc6c

Implement plates_attributes for Traction message

d55759d

Fix linting issues

b1f8cd7

Merge pull request #59 from sanger/dpl-952-pre-issue-refactor

a62d43d

DPL-952: Refactor the code before applying the update for Traction V2

Merge branch 'develop' into dpl-952-support-traction-v2-api

d01db05

Update README.md

972022d

Add a test for multiple plates appearing in the same message

a40196f

Add test for a mix of plates and tubes in traction messages

6156a40

Fix some documentation inaccuracies

7044f67

Remove unneeded repetition for request_payload

608340e

Merge pull request #60 from sanger/dpl-952-support-traction-v2-api

8a07c3b

DPL-952: Support traction V2 API

Remove unneeded blank lines in code block

83eb997

sdjmchattie requested review from yoldas, stevieing and BenTopping February 14, 2024 16:13

sdjmchattie self-assigned this Feb 14, 2024

sdjmchattie added 2 commits February 14, 2024 16:13

Update release version to 0.4.0

115fa05

Set version to 1.0.0

d7e226f

sdjmchattie changed the title ~~Prepare release of version 0.4.0~~ Prepare release of version 1.0.0 Feb 15, 2024

stevieing approved these changes Feb 15, 2024

View reviewed changes

yoldas reviewed Feb 15, 2024

View reviewed changes

yoldas approved these changes Feb 15, 2024

View reviewed changes

sdjmchattie and others added 3 commits February 15, 2024 16:41

Simplify location creation code

cd8101e

Remove the padding method and turn the location creation into a list comprehension

Use datetime.utcnow() in tests

d6c81b1

Not strictly necessary, but it's better practice than the locale affected now() method

Merge pull request #65 from sanger/dpl-952-remaining-fixes

0c040f5

DPL-952: Remaining Fixes

sdjmchattie merged commit 0d750fe into master Feb 15, 2024
10 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prepare release of version 1.0.0 #64

Prepare release of version 1.0.0 #64

sdjmchattie commented Feb 14, 2024

yoldas Feb 15, 2024

sdjmchattie Feb 15, 2024 •

edited

Loading

yoldas Feb 15, 2024

sdjmchattie Feb 15, 2024

yoldas Feb 15, 2024

sdjmchattie Feb 15, 2024

yoldas Feb 15, 2024

sdjmchattie Feb 15, 2024

yoldas left a comment

		@@ -79,7 +83,7 @@ def test_can_generate_correct_payload(self, valid_traction_qc_message):
		"shearing_qc_comments": "Comments",
		"date_submitted": datetime.now().timestamp() * 1000,

		@@ -44,24 +44,24 @@ def pad_number(self, number: int) -> str:
		padded_number = f"0{padded_number}"

Prepare release of version 1.0.0 #64

Prepare release of version 1.0.0 #64

Conversation

sdjmchattie commented Feb 14, 2024

Changes proposed in this pull request

Instructions for Reviewers

yoldas Feb 15, 2024

Choose a reason for hiding this comment

sdjmchattie Feb 15, 2024 • edited Loading

Choose a reason for hiding this comment

yoldas Feb 15, 2024

Choose a reason for hiding this comment

sdjmchattie Feb 15, 2024

Choose a reason for hiding this comment

yoldas Feb 15, 2024

Choose a reason for hiding this comment

sdjmchattie Feb 15, 2024

Choose a reason for hiding this comment

yoldas Feb 15, 2024

Choose a reason for hiding this comment

sdjmchattie Feb 15, 2024

Choose a reason for hiding this comment

yoldas left a comment

Choose a reason for hiding this comment

sdjmchattie Feb 15, 2024 •

edited

Loading