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.

Sign up for GitHub

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

Jump to bottom

Clean up design.rowland #113

Open
hamogu opened this issue Jan 6, 2017 · 2 comments · Fixed by #138
Open

Clean up design.rowland #113

hamogu opened this issue Jan 6, 2017 · 2 comments · Fixed by #138
Milestone
1.0

Comments

@hamogu
Copy link
Member

hamogu commented Jan 6, 2017

It sees that there are two 1-D and 2 2D structures with arbitrary inheritance hirachy. That can be simplified before a release.
@hamogu hamogu added this to the 1.0 milestone Jan 6, 2017
hamogu added a commit to hamogu/marxs that referenced this issue Apr 10, 2017
@hamogu
Document all placement methids on the Rowland torus.
d17f746 
This closes Chandra-MARX#113. It does not really solve the problem, but adds sufficient documentation.

At this point, I didn ot see an easy way to simplify the classes while stil providing all the fuctionailty that I want,
but this may be revisited later.
@hamogu hamogu mentioned this issue Apr 10, 2017
Merged
hamogu added a commit to hamogu/marxs that referenced this issue Apr 13, 2017
@hamogu
Document all placement methids on the Rowland torus.
dfbdd44 
This closes Chandra-MARX#113. It does not really solve the problem, but adds sufficient documentation.

At this point, I didn ot see an easy way to simplify the classes while stil providing all the fuctionailty that I want,
but this may be revisited later.
@hamogu
Copy link
Member Author

hamogu commented Jul 24, 2017

I've just run into a problem that resulted from the fact that the GratingArrayStructure has a default normal_spec. However, it's not as one might expect for the grating structure [0,0,0,1] which would be appropriate for rays focussed to the centre, instead, it's derived from the parent class, where it is set for to rowland_normal. Setting it normal to the Rowland circle makes sense for e.g. CCD detectors on the Rowland circle but not for gratings.
I think this needs to be resolved to make the defaults sensible. Either I could change it in GratingArrayStructure, but maybe it would be easier to not have a hierarchy, but mostly independent classes with different use cases.
The functions "place_on_radius" etc could be part of a mixin class if I want to avoid copy and pasting them.

@hamogu hamogu reopened this Jul 24, 2017
@hamogu
Copy link
Member Author

hamogu commented Aug 5, 2022

Those classes also have very different test coverage, from non-exitant to pretty good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.0
Development

Successfully merging a pull request may close this issue.

Document all placement methids on the Rowland torus. hamogu/marxs
1 participant
@hamogu