Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -90,20 +90,20 @@ This ensures regular users of the library do not need to install these dependenc
The `generate.py` script takes as input JSON as produced by the instruments endpoint:

```bash
codegen/lco/generator.py instruments.json
codegen/lco/generator.py {facility} instruments.json
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you this is a terrible omission

```

Or directly from stdin using a pipe:

```bash
curl https://observe.lco.global/api/instruments/ | codegen/lco/generator.py
curl https://observe.lco.global/api/instruments/ | codegen/lco/generator.py {facility}
```

If the output looks satisfactory, you can redirect the output to overwrite the
LCO instruments definition file:

```bash
curl https://observe.lco.global/api/instruments/ | codegen/lco/generator.py > src/aeonlib/ocs/lco/instruments.py
curl https://observe.lco.global/api/instruments/ | codegen/lco/generator.py {facility} > src/aeonlib/ocs/lco/instruments.py
```
# Supported Facilities

Expand Down
11 changes: 11 additions & 0 deletions codegen/lco/generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,14 @@
VALID_FACILITIES = ["SOAR", "LCO", "SAAO", "BLANCO"]


def extract_default(properties):
if properties['type'] == 'integer' or properties['type'] == 'float' or properties['type'] == 'boolean':
return properties['default']
elif properties['type'] == 'string':
return f'"{properties['default']}"'
return Any


def get_modes(ins: dict[str, Any], type: str) -> list[str]:
try:
return [m["code"] for m in ins["modes"][type]["modes"]]
Expand All @@ -37,6 +45,7 @@ def generate_instrument_configs(ins_s: str, facility: str) -> str:
trim_blocks=True,
lstrip_blocks=True,
)
j_env.filters['extract_default'] = extract_default
template = j_env.get_template("instruments.jinja")
ins_data = json.loads(ins_s)
instruments: list[dict[str, Any]] = []
Expand Down Expand Up @@ -84,6 +93,8 @@ def generate_instrument_configs(ins_s: str, facility: str) -> str:
k.rstrip("s"): v
for k, v in ins["optical_elements"].items()
},
"configuration_extra_params": ins['validation_schema'].get('extra_params', {}).get('schema', {}),
"instrument_config_extra_params": ins['validation_schema'].get('instrument_configs', {}).get('schema', {}).get('schema', {}).get('extra_params', {}).get('schema', {})
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I really don't like how brittle this is. I know you think I was over-engineering it, but I was hoping for a more generic ability to iterate over fields and check for the existence of validate_schema for an object, not just hardcoding the json path for a specific instance of it. But maybe this is how it has to be done.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The validation_schema on the instrument_type is applied at the configuration level of our request language. What you call brittle here is just me either returning the proper extra_params fields added for configuration or instrument_configuration (the most common ones used), or returning {} if none exists in the validation_schema. I'm not sure how that is brittle?

}
)

Expand Down
29 changes: 24 additions & 5 deletions codegen/lco/templates/instruments.jinja
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
# pyright: reportUnannotatedClassAttribute=false
# This file is generated automatically and should not be edited by hand.

from typing import Any, Annotated, Literal
from typing import Any, Annotated, Literal, Optional

from annotated_types import Le
from pydantic import BaseModel, ConfigDict
from annotated_types import Le, Ge
from pydantic import BaseModel, ConfigDict, Field
from pydantic.types import NonNegativeInt, PositiveInt

from aeonlib.models import TARGET_TYPES
Expand All @@ -13,6 +13,25 @@ from aeonlib.ocs.config_models import Roi


{% for ctx in instruments %}

class {{ ctx.class_name}}ConfigExtraParams(BaseModel):
model_config = ConfigDict(validate_assignment=True, extra='allow')
{% for field, properties in ctx.configuration_extra_params.items() %}
{% set optional = ('required' not in properties or not properties.required) and 'default' not in properties %}
{{ field }}: {% if optional %}Optional[{% endif %}{% if 'allowed' in properties %}Literal[{{ properties.allowed }}]{% else %}Annotated[{% if properties.type == 'string' %}str{% elif properties.type == 'integer' %}int{% elif properties.type == 'float' %}float{% elif properties.type == 'boolean' %}bool{% endif %}{% if 'min' in properties %}, Ge({{ properties.min }}){% endif %}{% if 'max' in properties %}, Le({{ properties.max }}){% endif %}]{% endif %}{% if optional %}] = None{% elif 'default' in properties %} = {{ properties | extract_default }}{% endif %}

{% endfor %}


class {{ ctx.class_name}}InstrumentConfigExtraParams(BaseModel):
model_config = ConfigDict(validate_assignment=True, extra='allow')
{% for field, properties in ctx.instrument_config_extra_params.items() %}
{% set optional = ('required' not in properties or not properties.required) and 'default' not in properties %}
{{ field }}: {% if optional %}Optional[{% endif %}{% if 'allowed' in properties %}Literal[{{ properties.allowed }}]{% else %}Annotated[{% if properties.type == 'string' %}str{% elif properties.type == 'integer' %}int{% elif properties.type == 'float' %}float{% elif properties.type == 'boolean' %}bool{% endif %}{% if 'min' in properties %}, Ge({{ properties.min }}){% endif %}{% if 'max' in properties %}, Le({{ properties.max }}){% endif %}]{% endif %}{% if optional %}] = None{% elif 'default' in properties %} = {{ properties | extract_default }}{% endif %}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this 500 column line the same as the 500 column line above? It's near impossible to read. Is there no way to factor this out? I was hoping this could be done in Python, like using a string builder pattern or something? Now you see why I was reaching for a parser...

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yes they are exactly the same - in fact the configuration and instrument configuration extra params sections are exactly the same except what they are iterating over. I don't know enough about jinja2 to figure out if you can wrap this in a template function or something to reuse. I thought a lot of this had to be done in the template and not in a python function because its using classes and stuff - the same reason you are using a jinja2 template at all to generate this?


{% endfor %}


class {{ ctx.class_name }}OpticalElements(BaseModel):
model_config = ConfigDict(validate_assignment=True)
{% for key, values in ctx.optical_elements.items() %}
Expand Down Expand Up @@ -49,7 +68,7 @@ class {{ ctx.class_name }}Config(BaseModel):
rotator_mode: Literal[{% for m in ctx.rotator_modes %}"{{ m }}"{% if not loop.last %}, {% endif %}{% endfor %}]
{% endif %}
rois: list[Roi] | None = None
extra_params: dict[Any, Any] = {}
extra_params: {{ ctx.class_name }}InstrumentConfigExtraParams = Field(default_factory={{ ctx.class_name }}InstrumentConfigExtraParams)
optical_elements: {{ ctx.class_name}}OpticalElements


Expand All @@ -58,7 +77,7 @@ class {{ ctx.class_name }}(BaseModel):
type: Literal[{% for t in ctx.config_types %}"{{ t }}"{% if not loop.last %}, {% endif %}{% endfor %}]
instrument_type: Literal["{{ ctx.instrument_type }}"] = "{{ ctx.instrument_type }}"
repeat_duration: NonNegativeInt | None = None
extra_params: dict[Any, Any] = {}
extra_params: {{ ctx.class_name }}ConfigExtraParams = Field(default_factory={{ ctx.class_name }}ConfigExtraParams)
instrument_configs: list[{{ ctx.class_name }}Config] = []
acquisition_config: {{ ctx.class_name }}AcquisitionConfig
guiding_config: {{ ctx.class_name }}GuidingConfig
Expand Down
25 changes: 20 additions & 5 deletions src/aeonlib/ocs/blanco/instruments.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,32 @@
# pyright: reportUnannotatedClassAttribute=false
# This file is generated automatically and should not be edited by hand.

from typing import Any, Annotated, Literal
from typing import Any, Annotated, Literal, Optional

Check failure on line 4 in src/aeonlib/ocs/blanco/instruments.py

View workflow job for this annotation

GitHub Actions / lint

Ruff (F401)

src/aeonlib/ocs/blanco/instruments.py:4:45: F401 `typing.Optional` imported but unused
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I can see this being a pain because this is generated code, and if there doesn't happen to be any optional attributes then this warning doesn't make sense.

Luckily, using Optional is discouraged. The correct way to annotate optional types now is to use Type | None like defocus: float | None = None

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

That's good to know, I was figuring out pydantic and jinja2 as i went...


from annotated_types import Le
from pydantic import BaseModel, ConfigDict
from annotated_types import Le, Ge
from pydantic import BaseModel, ConfigDict, Field
from pydantic.types import NonNegativeInt, PositiveInt

from aeonlib.models import TARGET_TYPES
from aeonlib.ocs.target_models import Constraints
from aeonlib.ocs.config_models import Roi



class BlancoNewfirmConfigExtraParams(BaseModel):
model_config = ConfigDict(validate_assignment=True, extra='allow')
dither_value: Annotated[int, Ge(0), Le(1600)] = 80
dither_sequence: Literal[['2x2', '3x3', '4x4', '5-point']] = "2x2"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Invalid syntax, There's an extra set of [] in there.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

oops, ill remove that

detector_centering: Literal[['none', 'det_1', 'det_2', 'det_3', 'det_4']] = "det_1"
dither_sequence_random_offset: Literal[[True, False]] = True
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Isn't this... "Literally" a boolean? ✔️

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

yes, but it's defined this way in the cerberus schema to get us a select dropdown in the UI... The effect in the library should be the same as if it was an Annotated[bool] right?



class BlancoNewfirmInstrumentConfigExtraParams(BaseModel):
model_config = ConfigDict(validate_assignment=True, extra='allow')
coadds: Annotated[int, Ge(1), Le(100)] = 1
sequence_repeats: Annotated[int, Ge(1), Le(500)] = 1


class BlancoNewfirmOpticalElements(BaseModel):
model_config = ConfigDict(validate_assignment=True)
filter: Literal["JX", "HX", "KXs"]
Expand Down Expand Up @@ -43,7 +58,7 @@
""" Exposure time in seconds"""
mode: Literal["fowler1", "fowler2"]
rois: list[Roi] | None = None
extra_params: dict[Any, Any] = {}
extra_params: BlancoNewfirmInstrumentConfigExtraParams = Field(default_factory=BlancoNewfirmInstrumentConfigExtraParams)
optical_elements: BlancoNewfirmOpticalElements


Expand All @@ -52,7 +67,7 @@
type: Literal["EXPOSE", "SKY_FLAT", "STANDARD", "DARK"]
instrument_type: Literal["BLANCO_NEWFIRM"] = "BLANCO_NEWFIRM"
repeat_duration: NonNegativeInt | None = None
extra_params: dict[Any, Any] = {}
extra_params: BlancoNewfirmConfigExtraParams = Field(default_factory=BlancoNewfirmConfigExtraParams)
instrument_configs: list[BlancoNewfirmConfig] = []
acquisition_config: BlancoNewfirmAcquisitionConfig
guiding_config: BlancoNewfirmGuidingConfig
Expand Down
78 changes: 65 additions & 13 deletions src/aeonlib/ocs/lco/instruments.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,29 @@
# pyright: reportUnannotatedClassAttribute=false
# This file is generated automatically and should not be edited by hand.

from typing import Any, Annotated, Literal
from typing import Any, Annotated, Literal, Optional

from annotated_types import Le
from pydantic import BaseModel, ConfigDict
from annotated_types import Le, Ge
from pydantic import BaseModel, ConfigDict, Field
from pydantic.types import NonNegativeInt, PositiveInt

from aeonlib.models import TARGET_TYPES
from aeonlib.ocs.target_models import Constraints
from aeonlib.ocs.config_models import Roi



class Lco0M4ScicamQhy600ConfigExtraParams(BaseModel):
model_config = ConfigDict(validate_assignment=True, extra='allow')
sub_expose: Literal[[False, True]] = False
sub_exposure_time: Optional[Annotated[float, Ge(15.0)]] = None
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As per previous comment, this should be sub_exposure_time: Annotated[float, Ge(15.0)] | None = None



class Lco0M4ScicamQhy600InstrumentConfigExtraParams(BaseModel):
model_config = ConfigDict(validate_assignment=True, extra='allow')
defocus: Optional[Annotated[float, Ge(-5.0), Le(5.0)]] = None


class Lco0M4ScicamQhy600OpticalElements(BaseModel):
model_config = ConfigDict(validate_assignment=True)
filter: Literal["OIII", "SII", "Astrodon-Exo", "w", "opaque", "up", "rp", "ip", "gp", "zs", "V", "B", "H-Alpha"]
Expand Down Expand Up @@ -43,7 +55,7 @@ class Lco0M4ScicamQhy600Config(BaseModel):
""" Exposure time in seconds"""
mode: Literal["central30x30", "full_frame"]
rois: list[Roi] | None = None
extra_params: dict[Any, Any] = {}
extra_params: Lco0M4ScicamQhy600InstrumentConfigExtraParams = Field(default_factory=Lco0M4ScicamQhy600InstrumentConfigExtraParams)
optical_elements: Lco0M4ScicamQhy600OpticalElements


Expand All @@ -52,7 +64,7 @@ class Lco0M4ScicamQhy600(BaseModel):
type: Literal["EXPOSE", "REPEAT_EXPOSE", "AUTO_FOCUS", "BIAS", "DARK", "STANDARD", "SKY_FLAT"]
instrument_type: Literal["0M4-SCICAM-QHY600"] = "0M4-SCICAM-QHY600"
repeat_duration: NonNegativeInt | None = None
extra_params: dict[Any, Any] = {}
extra_params: Lco0M4ScicamQhy600ConfigExtraParams = Field(default_factory=Lco0M4ScicamQhy600ConfigExtraParams)
instrument_configs: list[Lco0M4ScicamQhy600Config] = []
acquisition_config: Lco0M4ScicamQhy600AcquisitionConfig
guiding_config: Lco0M4ScicamQhy600GuidingConfig
Expand All @@ -65,6 +77,16 @@ class Lco0M4ScicamQhy600(BaseModel):
optical_elements_class = Lco0M4ScicamQhy600OpticalElements



class Lco1M0NresScicamConfigExtraParams(BaseModel):
model_config = ConfigDict(validate_assignment=True, extra='allow')


class Lco1M0NresScicamInstrumentConfigExtraParams(BaseModel):
model_config = ConfigDict(validate_assignment=True, extra='allow')
defocus: Optional[Annotated[float, Ge(-5.0), Le(5.0)]] = None


class Lco1M0NresScicamOpticalElements(BaseModel):
model_config = ConfigDict(validate_assignment=True)

Expand Down Expand Up @@ -95,7 +117,7 @@ class Lco1M0NresScicamConfig(BaseModel):
""" Exposure time in seconds"""
mode: Literal["default"]
rois: list[Roi] | None = None
extra_params: dict[Any, Any] = {}
extra_params: Lco1M0NresScicamInstrumentConfigExtraParams = Field(default_factory=Lco1M0NresScicamInstrumentConfigExtraParams)
optical_elements: Lco1M0NresScicamOpticalElements


Expand All @@ -104,7 +126,7 @@ class Lco1M0NresScicam(BaseModel):
type: Literal["NRES_SPECTRUM", "REPEAT_NRES_SPECTRUM", "NRES_EXPOSE", "NRES_TEST", "SCRIPT", "ENGINEERING", "ARC", "LAMP_FLAT", "NRES_BIAS", "NRES_DARK", "AUTO_FOCUS"]
instrument_type: Literal["1M0-NRES-SCICAM"] = "1M0-NRES-SCICAM"
repeat_duration: NonNegativeInt | None = None
extra_params: dict[Any, Any] = {}
extra_params: Lco1M0NresScicamConfigExtraParams = Field(default_factory=Lco1M0NresScicamConfigExtraParams)
instrument_configs: list[Lco1M0NresScicamConfig] = []
acquisition_config: Lco1M0NresScicamAcquisitionConfig
guiding_config: Lco1M0NresScicamGuidingConfig
Expand All @@ -117,6 +139,16 @@ class Lco1M0NresScicam(BaseModel):
optical_elements_class = Lco1M0NresScicamOpticalElements



class Lco1M0ScicamSinistroConfigExtraParams(BaseModel):
model_config = ConfigDict(validate_assignment=True, extra='allow')


class Lco1M0ScicamSinistroInstrumentConfigExtraParams(BaseModel):
model_config = ConfigDict(validate_assignment=True, extra='allow')
defocus: Optional[Annotated[float, Ge(-5.0), Le(5.0)]] = None


class Lco1M0ScicamSinistroOpticalElements(BaseModel):
model_config = ConfigDict(validate_assignment=True)
filter: Literal["I", "R", "U", "w", "Y", "up", "rp", "ip", "gp", "zs", "V", "B", "400um-Pinhole", "150um-Pinhole", "CN"]
Expand Down Expand Up @@ -148,7 +180,7 @@ class Lco1M0ScicamSinistroConfig(BaseModel):
""" Exposure time in seconds"""
mode: Literal["full_frame", "central_2k_2x2"]
rois: list[Roi] | None = None
extra_params: dict[Any, Any] = {}
extra_params: Lco1M0ScicamSinistroInstrumentConfigExtraParams = Field(default_factory=Lco1M0ScicamSinistroInstrumentConfigExtraParams)
optical_elements: Lco1M0ScicamSinistroOpticalElements


Expand All @@ -157,7 +189,7 @@ class Lco1M0ScicamSinistro(BaseModel):
type: Literal["EXPOSE", "REPEAT_EXPOSE", "BIAS", "DARK", "STANDARD", "SCRIPT", "AUTO_FOCUS", "ENGINEERING", "SKY_FLAT"]
instrument_type: Literal["1M0-SCICAM-SINISTRO"] = "1M0-SCICAM-SINISTRO"
repeat_duration: NonNegativeInt | None = None
extra_params: dict[Any, Any] = {}
extra_params: Lco1M0ScicamSinistroConfigExtraParams = Field(default_factory=Lco1M0ScicamSinistroConfigExtraParams)
instrument_configs: list[Lco1M0ScicamSinistroConfig] = []
acquisition_config: Lco1M0ScicamSinistroAcquisitionConfig
guiding_config: Lco1M0ScicamSinistroGuidingConfig
Expand All @@ -170,6 +202,16 @@ class Lco1M0ScicamSinistro(BaseModel):
optical_elements_class = Lco1M0ScicamSinistroOpticalElements



class Lco2M0FloydsScicamConfigExtraParams(BaseModel):
model_config = ConfigDict(validate_assignment=True, extra='allow')


class Lco2M0FloydsScicamInstrumentConfigExtraParams(BaseModel):
model_config = ConfigDict(validate_assignment=True, extra='allow')
defocus: Optional[Annotated[float, Ge(-5.0), Le(5.0)]] = None


class Lco2M0FloydsScicamOpticalElements(BaseModel):
model_config = ConfigDict(validate_assignment=True)
slit: Literal["slit_6.0as", "slit_1.6as", "slit_2.0as", "slit_1.2as"]
Expand Down Expand Up @@ -202,7 +244,7 @@ class Lco2M0FloydsScicamConfig(BaseModel):
mode: Literal["default"]
rotator_mode: Literal["VFLOAT", "SKY"]
rois: list[Roi] | None = None
extra_params: dict[Any, Any] = {}
extra_params: Lco2M0FloydsScicamInstrumentConfigExtraParams = Field(default_factory=Lco2M0FloydsScicamInstrumentConfigExtraParams)
optical_elements: Lco2M0FloydsScicamOpticalElements


Expand All @@ -211,7 +253,7 @@ class Lco2M0FloydsScicam(BaseModel):
type: Literal["SPECTRUM", "REPEAT_SPECTRUM", "ARC", "ENGINEERING", "SCRIPT", "LAMP_FLAT"]
instrument_type: Literal["2M0-FLOYDS-SCICAM"] = "2M0-FLOYDS-SCICAM"
repeat_duration: NonNegativeInt | None = None
extra_params: dict[Any, Any] = {}
extra_params: Lco2M0FloydsScicamConfigExtraParams = Field(default_factory=Lco2M0FloydsScicamConfigExtraParams)
instrument_configs: list[Lco2M0FloydsScicamConfig] = []
acquisition_config: Lco2M0FloydsScicamAcquisitionConfig
guiding_config: Lco2M0FloydsScicamGuidingConfig
Expand All @@ -224,6 +266,16 @@ class Lco2M0FloydsScicam(BaseModel):
optical_elements_class = Lco2M0FloydsScicamOpticalElements



class Lco2M0ScicamMuscatConfigExtraParams(BaseModel):
model_config = ConfigDict(validate_assignment=True, extra='allow')


class Lco2M0ScicamMuscatInstrumentConfigExtraParams(BaseModel):
model_config = ConfigDict(validate_assignment=True, extra='allow')
defocus: Optional[Annotated[float, Ge(-8.0), Le(8.0)]] = None


class Lco2M0ScicamMuscatOpticalElements(BaseModel):
model_config = ConfigDict(validate_assignment=True)
narrowband_g_position: Literal["out", "in"]
Expand Down Expand Up @@ -258,7 +310,7 @@ class Lco2M0ScicamMuscatConfig(BaseModel):
""" Exposure time in seconds"""
mode: Literal["MUSCAT_SLOW", "MUSCAT_FAST"]
rois: list[Roi] | None = None
extra_params: dict[Any, Any] = {}
extra_params: Lco2M0ScicamMuscatInstrumentConfigExtraParams = Field(default_factory=Lco2M0ScicamMuscatInstrumentConfigExtraParams)
optical_elements: Lco2M0ScicamMuscatOpticalElements


Expand All @@ -267,7 +319,7 @@ class Lco2M0ScicamMuscat(BaseModel):
type: Literal["EXPOSE", "REPEAT_EXPOSE", "BIAS", "DARK", "STANDARD", "SCRIPT", "AUTO_FOCUS", "ENGINEERING", "SKY_FLAT"]
instrument_type: Literal["2M0-SCICAM-MUSCAT"] = "2M0-SCICAM-MUSCAT"
repeat_duration: NonNegativeInt | None = None
extra_params: dict[Any, Any] = {}
extra_params: Lco2M0ScicamMuscatConfigExtraParams = Field(default_factory=Lco2M0ScicamMuscatConfigExtraParams)
instrument_configs: list[Lco2M0ScicamMuscatConfig] = []
acquisition_config: Lco2M0ScicamMuscatAcquisitionConfig
guiding_config: Lco2M0ScicamMuscatGuidingConfig
Expand Down
Loading
Loading