Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Logging utils no longer exported #550

Open
DiamondJoseph opened this issue Aug 30, 2024 · 6 comments
Open

Logging utils no longer exported #550

DiamondJoseph opened this issue Aug 30, 2024 · 6 comments

Comments

@DiamondJoseph
Copy link
Contributor

No description provided.

@coretl
Copy link
Collaborator

coretl commented Aug 30, 2024

Which ones in particular?

@DiamondJoseph
Copy link
Contributor Author

dodal was using

from ophyd_async.log import (
    DEFAULT_DATE_FORMAT,
    DEFAULT_FORMAT,
    DEFAULT_LOG_COLORS,
    ColoredFormatterWithDeviceName,
)
from ophyd_async.log import logger as ophyd_async_logger

None of these are exported in 0.5.0

ophyd_async_logger was used to configure the logging level of ophyd-async and set it as part of a logging hierarchy
The rest were used to construct a DEFAULT_FORMATTER to make logging consistent between all loggers there (graylog, file, stdout)

@coretl
Copy link
Collaborator

coretl commented Sep 2, 2024

@abbiemery any opinions?

@DiamondJoseph
Copy link
Contributor Author

DiamondJoseph commented Sep 2, 2024

While I'm in this issue, other things that dodal was using but that are no longer private:

merge_gathered_dicts, SoftConverter,

@DiamondJoseph
Copy link
Contributor Author

class ApertureConverter(SoftConverter):
        # Ophyd-async #311 should add a default converter for dataclasses to do this
        def reading(
            self, value: SingleAperturePosition, timestamp: float, severity: int
        ) -> Reading:
            return Reading(
                value=asdict(value),
                timestamp=timestamp,
                alarm_severity=-1 if severity > 2 else severity,
            )

SoftConverter was being overriden to allow returning a SingleAperturePosition, a dataclass, which I believe would not actually be allowed as it's not returning one of the types allowed by DataKey? I can only assume this was required previously for it to work and it was working.

@dataclass
class SingleAperturePosition:
    name: str
    GDA_name: str
    radius_microns: float | None
    location: ApertureFiveDimensionalLocation

merge_gathered_dicts was being used to gather softsignal metadata for devices

@coretl
Copy link
Collaborator

coretl commented Sep 2, 2024

@DominicOram says that for this individual use case "if it is causing pain, then it's ok to explode it out into multiple signals", which should mean you don't need SoftConverter or merge_gathered_dicts, and it won't be broken by #310

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants