Feat 239 Update Smartspim Imaging ETL #281

mekhlakapoor · 2025-04-24T23:05:29Z

closes #239

This PR:

updates the SmartSPIM Imaging ETL to match Mike's script
removes mapping data from processing manifest in the ETL (left it in AcquisitionJobSettings for backwards compatibility but it will not be used!)
integrates data from SLIMS into Acquisition model (in _transform method)
Adds example_acquisition.json and tests. If the example is too large, we can remove it

Example usage:

from aind_metadata_mapper.smartspim.models import JobSettings
from aind_metadata_mapper.smartspim.acquisition import SmartspimETL

settings = JobSettings(
subject_id="786846",
 metadata_service_domain="http://aind-metadata-service", 
input_source="input_dir", output_directory="."
)
etl = SmartspimETL(settings)
etl.run_job() # Writes etl to output dir

mekhlakapoor · 2025-04-25T00:10:18Z

src/aind_metadata_mapper/smartspim/utils.py

-    x_res = y_res = session_config.get("µm/pix")
-    z_res = session_config.get("z_step_um")
+    x_res = y_res = session_config.get("um/pix")
+    z_res = session_config.get("Z step (um)")


The key in session_config for "Z step" was different in Mike's script so I updated it here. Let me know if this shouldnt be changed

I don't think we should change it. @miketaormina correct me if I am wrong, but these keys depend on the version of the microscope software dataset we are going to upload. I think we can stay with the latest since we have already uploaded old datasets.

Yeah, this is really confusing. It looks like

the original JSON/TXT files it is Z step (um), which depends on the version like @camilolaiton says.

The function called on a JSON version at this line renames this key to z_step_um. This then works without this proposed change.

In the event that the JSON file doesn't exist, the script reads the .txt file and I think would fail unless the proposed change were implemented.

In short, the code should keep the z_res = session_config.get("z_step_um") and we need to better handle the reading of a TXT file. BUT I'm not sure how this is working with the $\mu$ symbol. When I look at a session_config object created with the current script, I see an entry 'm/pix': '1.800'

The function that reads the JSON is here, which isn't what I expected. . . I know we've had a bout 4 different ways of dealing with this weird encoding examples

Sorry this is so confusing @mekhlakapoor , but I think that this current PR should keep z_step_um but change µm/pix to um/pix

This gets us to a presumbly working state and the issue with reading a txt instead of JSON can be a. separate issue on the repo

mekhlakapoor · 2025-04-25T21:51:04Z

src/aind_metadata_mapper/smartspim/models.py

+        default="derivatives/processing_manifest.json",
+        description=("Deprecated, use metadata_service_path instead. "),
+    )
+    metadata_service_domain: str


separated metadata_service_domain and metadata_service_path to match pattern of GatherMetadata JobSettings. However, that might be unnecessary so let me know and I can just make it one with no default, or default to "http://aind-metadata-service/slims/smartspim_imaging

camilolaiton · 2025-05-06T19:29:27Z

src/aind_metadata_mapper/smartspim/models.py

+class SlimsImmersionMedium(Enum):
+    """Enum for the immersion medium used in SLIMS."""
+
+    DIH2O = "diH2O"


Is there a way in how we can do something like aind-data-schema-models where we can simply have a CSV? @miketaormina do you think we will upgrade/change the mediums in the future?

The enum was to validate the values coming out of SLIMS and map it, but we can do a str check instead

Enumerating the Cargille oil like this is weird for two reasons:

The catalogue number from the manufacturer is the same for both of these "1.53" and "1.52," they just have different values of refractive index.

The numerical value of refractive index is additionally measure in another field (or at least used to be).

Most acquisitions these days use an oil with RI=1.5170.

I think this is fine if it is what matches SLIMS, but that might mean that SLIMS needs updated. I thought that schema was built like that already (separating the immersion type from the RI value: schema

This enum is what is in SLIMS right now. (It puts them together). The response from SLIMS is then mapped to the schema (with the separation of immersion type and RI value). We can ask for SLIMS to be updated

camilolaiton

Almost everything looks good to me, except the z_step_um where I mentioned @miketaormina for his knowledge on the microscope side.

camilolaiton · 2025-05-06T19:38:11Z

src/aind_metadata_mapper/smartspim/utils.py

-    x_res = y_res = session_config.get("µm/pix")
-    z_res = session_config.get("z_step_um")
+    x_res = y_res = session_config.get("um/pix")
+    z_res = session_config.get("Z step (um)")


I don't think we should change it. @miketaormina correct me if I am wrong, but these keys depend on the version of the microscope software dataset we are going to upload. I think we can stay with the latest since we have already uploaded old datasets.

mekhlakapoor · 2025-05-09T17:50:21Z

src/aind_metadata_mapper/smartspim/utils.py

@@ -153,11 +160,13 @@ def make_acq_tiles(metadata_dict: dict, filter_mapping: dict):
            )

    if z_res is None:
-        z_res = session_config.get("Z step (m)")
+        z_res = session_config.get("Z step (um)")


@miketaormina @camilolaiton changed the unit here. Let me know if this was a bug or if it needs to be able to check for Z step (m) too

I think that the z_step_um was failing because the other repository changed the key here. So the file should read Z step (um), but was read into a dictionary as z_step_um.

But I think that the if statement here is meant to catch a different problem, namely that the text file encoding for the micron character has always been slippery, so there were cases where the character just gets dropped instead of replaced with a "u."

So I think we mislead you on the correct value of z_res. We need to either change that key in the JSON file, or change this repo to z_res = session_config.get("Z step (um)"), as you had before. In that scenario, line 163 would read z_res = session_config.get("Z step (m)") as a catch on the character encoding of µ

How about I check all 3? Z step (um), z_step_um, and then lastly Z step(m) ?

…namics/aind-metadata-mapper into feat-239-smartspim-imaging

miketaormina

I think the JSON key issue was solved (Z step (um) etc.). I am okay with the rest of this, if it has been tested as creating a valid acquisition object

miketaormina · 2025-05-12T21:09:12Z

src/aind_metadata_mapper/smartspim/models.py

+class SlimsImmersionMedium(Enum):
+    """Enum for the immersion medium used in SLIMS."""
+
+    DIH2O = "diH2O"


Enumerating the Cargille oil like this is weird for two reasons:

The catalogue number from the manufacturer is the same for both of these "1.53" and "1.52," they just have different values of refractive index.

The numerical value of refractive index is additionally measure in another field (or at least used to be).

Most acquisitions these days use an oil with RI=1.5170.

I think this is fine if it is what matches SLIMS, but that might mean that SLIMS needs updated. I thought that schema was built like that already (separating the immersion type from the RI value: schema

mekhlakapoor · 2025-05-19T23:39:19Z

Add an optional field in JobSettings for a user to define a "slims_date". As default, SLIMS should check against the microscope datetimes. If SLIMS date is supplied, check gte and lte slims_date

mekhlakapoor added 11 commits April 14, 2025 17:03

integrate smartspim imaging endpoint in acquistion (wip)

27469af

wip

29c9298

Merge branch 'dev' into feat-239-smartspim-imaging

1af4e5f

updates ETL

2c819ce

integrating slims response (wip)

b77007d

finish integrating slims data in ETL job

4d36015

clean up etl methods, tests (wip)

f201f85

unit tests for updated ETL

b35eefd

Merge branch 'dev' into feat-239-smartspim-imaging

154ec18

updates to dev, linters

4deb3f3

fix gather metadata tests

6a36c12

mekhlakapoor requested review from jtyoung84 and camilolaiton April 25, 2025 00:08

mekhlakapoor commented Apr 25, 2025

View reviewed changes

mekhlakapoor marked this pull request as draft April 25, 2025 20:26

start time fix

3d85f9c

mekhlakapoor marked this pull request as ready for review April 25, 2025 21:18

mekhlakapoor commented Apr 25, 2025

View reviewed changes

mekhlakapoor requested a review from miketaormina May 5, 2025 22:42

camilolaiton reviewed May 6, 2025

View reviewed changes

fix: reverts z step, adds support for Path/str inputs

a3d7261

mekhlakapoor marked this pull request as draft May 8, 2025 20:20

mekhlakapoor added 3 commits May 8, 2025 13:22

linter fix

28a2239

fix tests

cbc1954

fix in gather metadata tests

555c99c

mekhlakapoor marked this pull request as ready for review May 8, 2025 21:06

mekhlakapoor requested a review from camilolaiton May 8, 2025 21:07

check key Z step (um) if z_step_um is None

33de279

mekhlakapoor commented May 9, 2025

View reviewed changes

mekhlakapoor added 4 commits May 9, 2025 11:14

adds smartspim to pyproject.toml, readme

152a8ce

remove local readme

4767942

support all 3 z step options

ea13074

Merge branch 'feat-239-smartspim-imaging' of github.com:AllenNeuralDy…

9558928

…namics/aind-metadata-mapper into feat-239-smartspim-imaging

miketaormina approved these changes May 12, 2025

View reviewed changes

optional parameter for slims_datetime

92c9021

mekhlakapoor marked this pull request as draft May 20, 2025 23:03

mekhlakapoor added 3 commits May 20, 2025 16:07

linters

b1e826a

linters pt2

784f8fa

fix tests import

9bc1df3

mekhlakapoor marked this pull request as ready for review May 21, 2025 20:54

mekhlakapoor merged commit b8f4807 into dev May 27, 2025
3 checks passed

mekhlakapoor deleted the feat-239-smartspim-imaging branch May 27, 2025 16:47

Feat 239 Update Smartspim Imaging ETL #281

Feat 239 Update Smartspim Imaging ETL #281

Uh oh!

Conversation

mekhlakapoor commented Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

camilolaiton left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

miketaormina left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mekhlakapoor commented May 19, 2025

Uh oh!

Uh oh!

Uh oh!

mekhlakapoor commented Apr 24, 2025 •

edited

Loading