Galran/new emitters module #79
Conversation
… and CompositeSource data types and added a docmentation page for the new emitters. also added the using of the OpticSim.Emitters to the documentation script.
Codecov Report
@@ Coverage Diff @@
## main #79 +/- ##
==========================================
- Coverage 51.69% 49.01% -2.68%
==========================================
Files 54 55 +1
Lines 6092 6425 +333
==========================================
Hits 3149 3149
- Misses 2943 3276 +333
Continue to review full report at Codecov.
|
|
@galran let's bite the bullet and blow away the old emitter code and make the change to the new emitter code. It will be more difficult to do it later when people have started to use the old emitter api and have to rewrite code. |
|
@BrianGun Do you want to clean the old emitters code as part of this PR or in two steps. accept this one and do a "clean PR" separately? |
|
Let's do the merge in two steps. First add the new emitters but leave the old emitter code and examples, etc., then in a separate PR make all the examples and scripts compatible with the new emitter model. |
There was a problem hiding this comment.
@galran This module is named Emitters and the file is named Emitters.jl. Won't this replace the existing Emitters module and possibly cause issues with old code that uses the old interface? I thought we were going to do the update in two steps, first add the new Emitters.jl and then update the old code to match. Won't overwriting Emitters.jl automatically cause all the old code to break?
Also it might be a good idea to break Emitters.jl into multiple files. There's a lot of code in that one file. Or did you intend to do this in the second phase when all the old Emitter code is switched over to the new emitter code?
|
@BrianGun I don't think the file name is part of the Julia namespace in any way - i might be wrong. I can break the file into separate files and include them, but I think the content is separated using regions which make is esier to navigate. maybe i will break them into Emitters_Power.jl, Emitters_Origins.jl... |
This PR will add the new Emitters module to the package.
I’m reluctant to remove the old emitters implementation as they are used in multiple examples and copy snips and maybe scripts outside the package.
I would suggest adding the new module and over time removing the old implementation.
Currently, I’ve labeled the documentation page as “Emitters (NEW)” – I know it’s ugly but I don’t want to turn it into a huge package-wide change now.