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

Replace all remaining file_name with filename #165

Draft
wants to merge 1 commit into
base: dev_master
Choose a base branch
from

Conversation

teutoburg
Copy link
Contributor

I guess those were missed in #147? Anyway, they caused some errors and also a hard-to-trackdown weirdness of wrong detector layout (imaging layout was used in IFU mode). This fixed it.

@teutoburg teutoburg added the instrument-specific Limited to a certain IRDB instrument package label Mar 25, 2024
@teutoburg teutoburg self-assigned this Mar 25, 2024
@teutoburg teutoburg marked this pull request as ready for review March 25, 2024 16:09
Copy link
Collaborator

@hugobuddel hugobuddel left a comment

Choose a reason for hiding this comment

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

These file_names should not cause any problems. In fact, we changed some of these before (not sure whether in #147 or later), but we had to revert them (to get the IFU mode working).

The __init__() of effects should not have an file_name parameter, and therefor the kwargs in the yaml files should never have an file_name keyword them.

However, these file_names only appear in the properties, and thus only in the values of the kwargs in the yaml files, not the keys.

Now arguably, we can also change these to filename, and I would be fine with that. However, we should first make sure that we understand what we are doing, because these are self-consistent and should not have caused any "hard-to-trackdown weirdness".

In particular, the current ScopeSim code will be broken if we make this change now:

scopesim/effects/metis_lms_trace_list.py
310:        filename_det_layout = from_currsys("!DET.layout.file_name", cmds=self.cmds)

Could you elaborate on why this change is necessary?

@teutoburg teutoburg marked this pull request as draft August 29, 2024 08:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
instrument-specific Limited to a certain IRDB instrument package
Projects
Status: 🏗 In progress
Development

Successfully merging this pull request may close these issues.

2 participants