Skip to content

Add fileExtension to configuration query#54

Merged
tpoliaw merged 1 commit into
mainfrom
fallback_extension
Jan 20, 2025
Merged

Add fileExtension to configuration query#54
tpoliaw merged 1 commit into
mainfrom
fallback_extension

Conversation

@tpoliaw

@tpoliaw tpoliaw commented Jan 16, 2025

Copy link
Copy Markdown
Collaborator

Fixes #52

@tpoliaw

tpoliaw commented Jan 16, 2025

Copy link
Copy Markdown
Collaborator Author

More naming-is-difficult decisions. I'm not sure whether it should be fallbackExtension, fileExtension, extension, something else? 'fallback' doesn't really make sense when it's not really a fallback anymore. (file)extension sounds like it could be the extension used for scan files etc.

Thoughts @callumforrester, @DiamondJoseph ?

@callumforrester

Copy link
Copy Markdown
Contributor

My concern here is that we may be dealing with systems that decide their own file extensions rather than letting us decide for them. For example a detector that can only write TIFFs, or that we're just coupling ourselves to nexus when a beamline might want multiple archive formats (I think B23 is like this?), but I may be misunderstanding.

@tpoliaw

tpoliaw commented Jan 16, 2025

Copy link
Copy Markdown
Collaborator Author

but I may be misunderstanding

That's what I was worried about but I'm not sure what would make it less ambiguous. The file extension is nothing to do with data files being written. It is the extension of the number file GDA creates in its var directory, eg 12345.i22

@tpoliaw

tpoliaw commented Jan 16, 2025

Copy link
Copy Markdown
Collaborator Author

It's just called extension when setting it via the configuration mutation. Whatever this ends up being, they should probably be made to match.

@callumforrester

Copy link
Copy Markdown
Contributor

If it's always going to be a number then we could copy ophyd-async and AreaDetector and call it fileNumber: https://github.com/bluesky/ophyd-async/blob/efe14d8561914c358c5ed5c1c36e195223699402/src/ophyd_async/epics/adcore/_core_io.py#L138

@tpoliaw

tpoliaw commented Jan 20, 2025

Copy link
Copy Markdown
Collaborator Author

It's not going to be a number. In most cases it would be the beamline name, but occasionally it's something like scanbase_numtracker or tmp. I think we've settled on tracker_file_extension in #57.

@callumforrester callumforrester left a comment

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.

Happy with that then, although I'm not sure I have a sufficient understanding of how GDA var works to review this properly

To avoid the method name clash between graphQL object impl and 'normal'
impl, the tracker_file_extension field has been made public and accessed
directly wherever it's used.
@tpoliaw tpoliaw force-pushed the fallback_extension branch from dca1b14 to 3c7afc9 Compare January 20, 2025 13:36
@tpoliaw tpoliaw merged commit a855d38 into main Jan 20, 2025
@tpoliaw tpoliaw deleted the fallback_extension branch January 20, 2025 15:53
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

Successfully merging this pull request may close these issues.

Add fallbackExtension field to configuration

2 participants