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

There are two thermal modules #185

Closed
settwi opened this issue Jan 31, 2025 · 3 comments
Closed

There are two thermal modules #185

settwi opened this issue Jan 31, 2025 · 3 comments
Labels
Bug Probably a bug. Discussion An issue opened for, or undergoing discussion. legacy

Comments

@settwi
Copy link
Contributor

settwi commented Jan 31, 2025

Describe the bug

There are two thermal modules, one in sunkit_spex.legacy.thermal and one in sunkit_spex.models.physical.thermal.

Recently in #178 we patched some issues with the thermal emission continuum but only in the legacy module. So, things are starting to diverge between the two modules.

I guess, if things change in the new thermal emission version, it isn't really an issue because the legacy version will be independent. But, for now, if people want to use the thermal emission, it's unclear which should be used.

Should we move the legacy emission to the models.physical model and then just have the legacy version import the current one? Or, should it be the other way around?

@settwi settwi added Bug Probably a bug. legacy Discussion An issue opened for, or undergoing discussion. labels Jan 31, 2025
@natsat0919
Copy link
Contributor

Imo legacy should stay untouched and be independent from the refactored code (i.e no cross-imports). I think the best option is to apply the changes from #178 to sunkit_spex.models.physical.thermal as well. I can do that if needed

@settwi
Copy link
Contributor Author

settwi commented Feb 4, 2025

What if we just import legacy in the new module?

@samaloney
Copy link
Collaborator

But then if we need to change the thermal code for example to better integrate the astropy modelling API we have to update other that uses it in legacy. The way I look at it legacy basically a read only copy bar bug fixes.

@settwi settwi closed this as not planned Won't fix, can't repro, duplicate, stale Feb 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Probably a bug. Discussion An issue opened for, or undergoing discussion. legacy
Projects
None yet
Development

No branches or pull requests

3 participants