Migrate to copier #159

d-perl · 2024-08-15T13:51:22Z

To test:

Check docs are intact
Confirm changed tests are correct

codecov · 2024-08-15T13:52:52Z

Codecov Report

Attention: Patch coverage is 36.08247% with 124 lines in your changes missing coverage. Please review.

Project coverage is 57.70%. Comparing base (1928954) to head (f161198).
Report is 14 commits behind head on main.

Files	Patch %	Lines
...4/serial/fixed_target/i24ssx_Chip_Manager_py3v1.py	25.00%	45 Missing ⚠️
...luesky/i24/serial/setup_beamline/setup_beamline.py	40.90%	26 Missing ⚠️
...4/serial/fixed_target/i24ssx_Chip_Mapping_py3v1.py	0.00%	19 Missing ⚠️
...4/serial/fixed_target/i24ssx_Chip_StartUp_py3v1.py	33.33%	10 Missing ⚠️
src/mx_bluesky/i24/serial/dcid.py	0.00%	8 Missing ⚠️
...4/serial/fixed_target/i24ssx_Chip_Collect_py3v1.py	54.54%	5 Missing ⚠️
src/mx_bluesky/i24/serial/write_nexus.py	20.00%	4 Missing ⚠️
...4/serial/extruder/i24ssx_Extruder_Collect_py3v2.py	71.42%	2 Missing ⚠️
...sky/i24/serial/parameters/experiment_parameters.py	66.66%	2 Missing ⚠️
src/mx_bluesky/example.py	75.00%	1 Missing ⚠️
... and 2 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #159      +/-   ##
==========================================
+ Coverage   57.08%   57.70%   +0.62%     
==========================================
  Files          28       29       +1     
  Lines        3199     3256      +57     
==========================================
+ Hits         1826     1879      +53     
- Misses       1373     1377       +4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

DominicOram

Great, thank you! Couple of comments, some of which are more related to general code issues that to this change so feel free to ignore/spin out

.copier-answers.yml

DominicOram · 2024-08-16T10:03:38Z

.copier-answers.yml

+package_name: mx_bluesky
+pypi: true
+repo_name: mx_bluesky


Could: I can't find the discussion now but I think @callumforrester convinced me that we should be using hyphens rather than underscores in our package names. Is this a good opportunity to fix this here?

It's in PEP8

great, I wanted to do that but didn't want to have a fight about it

.github/workflows/ci.yml

DominicOram · 2024-08-16T10:13:47Z

Dockerfile

+# The devcontainer should use the developer target and run as root with podman
+# or docker with user namespaces.
+ARG PYTHON_VERSION=3.11
+FROM python:${PYTHON_VERSION} as developer


I made DiamondLightSource/python-copier-template#188 for this warning

DominicOram · 2024-08-16T10:41:09Z

src/mx_bluesky/i24/serial/fixed_target/i24ssx_Chip_Manager_py3v1.py

@@ -251,22 +248,24 @@ def save_screen_map() -> MsgGenerator:

 @log.log_on_entry
 def upload_parameters(
-    chipid: str = "oxford",
-    pmac: PMAC = inject("pmac"),
+    chipid: str = "oxford", pmac: PMAC = inject("pmac"), width: int | None = None


Could: It looks like upload_parameters is only ever called with oxford. If we don't use any other chips a better fix here would be to not even take the chip type as an argument and just hardcode oxford and width=8? Pull into a separate issue if we're not sure

For this and other similar, I didn't want to change things beyond the minimum necessary to make pyright happy here, but I agree these things all need cleaning up

DominicOram · 2024-08-16T10:56:57Z

src/mx_bluesky/i24/serial/fixed_target/i24ssx_Chip_Mapping_py3v1.py

@@ -79,36 +79,36 @@ def plot_file(fid, chip_type):
    ax1.set_xlim(-1, 26)
    ax1.set_ylim(-1, 26)
    ax1.invert_yaxis()
-    check_files(["%s.png" % chip_type])
-    plt.savefig("%s.png" % fid[:-5], dpi=200, bbox_inches="tight", pad_inches=0.05)
+    check_files("i24", [f"{chip_type}.png"])


Could: check_files only ever uses i24 as a location, maybe this because they change the hardcoded location to something else when they do it at the xfel? If that is the case we need a clean way for them to change it rather than have to go through every location? If not the case why pass a location to it at all. Maybe new issue?

DominicOram · 2024-08-16T11:01:05Z

src/mx_bluesky/i24/serial/fixed_target/i24ssx_Chip_StartUp_py3v1.py

    elif chip_type == ChipType.Custom:
+        # No fiducial for custom


Should: What does this comment give you over the existing log?

I didn't add this comment, just moved it down two lines so it was in the section it referred to

DominicOram · 2024-08-16T11:05:27Z

tests/i04/test_thawing.py

+    time: float | Literal[10] | Literal[50],
+    rotation: Literal[360] | Literal[100] | Literal[-100],
+    expected_speed: ApproxBase | Literal[72] | Literal[4],


Should: This odd, if I want to add another case to the parameterize I need to change these types? I feel like they should just be floats/ints?

hah, I guess ruff added these automagically

tests/i04/test_thawing.py

DominicOram · 2024-08-16T11:08:19Z

tests/i24/serial/setup_beamline/test_zebra_plans.py

@@ -29,7 +29,7 @@


 async def test_arm_and_disarm_zebra(zebra: Zebra, RE):
-    zebra.pc.arm.TIMEOUT = 1
+    zebra.pc.arm.TIMEOUT = 1  # type: ignore


Could: I feel like the correct answer is to change the definition to be TIMEOUT: float = 3

DiamondLightSource/dodal#750

DominicOram

Thanks, I think you were slightly too aggressive on the find/replace. Otherwise good.

docs/developer/serial-crystallography-on-i24/how-to/run-a-collection.rst

docs/developer/serial-crystallography-on-i24/how-to/stage-pmac-moves.rst

docs/user/reference/api.rst

DominicOram

Great, thank you!

d-perl added 5 commits August 15, 2024 11:03

adopt copier 1.0.0

9fcd898

update copier to 2.2.0

229e747

fix some settings and tests

34c3dcd

restore docs

6f29baf

Merge branch 'main' into 158_migrate_to_copier

ad7bc8f

d-perl added 9 commits August 15, 2024 14:58

pin p4p

532e94d

fix some ruff

9f30ae7

run ruff unsafe fixes to fix fstrings

d18790e

run precommit to add newlines

a0aff64

address more ruff

1d16784

remove 3.12 from CI tests

7160b03

fix pyright errors

bd7971d

fix tests

b33b29d

fix owner name

cb07290

d-perl requested a review from DominicOram August 15, 2024 15:22

d-perl added 3 commits August 15, 2024 17:03

Merge branch 'main' into 158_migrate_to_copier

4dc37bb

add newline

000f6e0

fix typing

2c44242

DominicOram requested changes Aug 16, 2024

View reviewed changes

give package name a hyphen

0a0d40d

This was referenced Aug 16, 2024

Remove unused arguments in upload_parameters() #160

Closed

Make a nice way to manage location in check_files #161

Open

d-perl added 3 commits August 16, 2024 13:36

remove typeignore

223cf8b

pin dodal

223d542

unpin dodal

f161198

d-perl requested a review from DominicOram August 16, 2024 13:00

replace reference to mx_bluesky in docs

ad47831

d-perl force-pushed the 158_migrate_to_copier branch from 8658b26 to ad47831 Compare August 16, 2024 13:05

DominicOram requested changes Aug 16, 2024

View reviewed changes

d-perl added 2 commits August 16, 2024 14:15

except where they refer to module...

156d168

and file paths...

b653b0b

d-perl requested a review from DominicOram August 16, 2024 13:41

DominicOram approved these changes Aug 16, 2024

View reviewed changes

d-perl merged commit 783badc into main Aug 19, 2024
18 checks passed

d-perl deleted the 158_migrate_to_copier branch August 19, 2024 08:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate to copier #159

Migrate to copier #159

d-perl commented Aug 15, 2024

codecov bot commented Aug 15, 2024 •

edited

Loading

DominicOram left a comment

DominicOram Aug 16, 2024

callumforrester Aug 16, 2024

d-perl Aug 16, 2024

DominicOram Aug 16, 2024

DominicOram Aug 16, 2024

d-perl Aug 16, 2024

d-perl Aug 16, 2024

DominicOram Aug 16, 2024

DominicOram Aug 16, 2024

d-perl Aug 16, 2024

DominicOram Aug 16, 2024

d-perl Aug 16, 2024

DominicOram Aug 16, 2024

d-perl Aug 16, 2024

DominicOram Aug 16, 2024

d-perl Aug 16, 2024

DominicOram left a comment

DominicOram left a comment

Migrate to copier #159

Migrate to copier #159

Conversation

d-perl commented Aug 15, 2024

codecov bot commented Aug 15, 2024 • edited Loading

Codecov Report

DominicOram 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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DominicOram left a comment

Choose a reason for hiding this comment

DominicOram left a comment

Choose a reason for hiding this comment

codecov bot commented Aug 15, 2024 •

edited

Loading