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

adding DRAGONS version to header of outputs #299

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

olyoberdorf
Copy link
Contributor

In _write_final, if we are saving an output file, add a header keyword recording the version of DRAGONS used

@KathleenLabrie
Copy link
Contributor

This will be added only at the end of the recipe. What about when I do writeOutputs() ? What about intermediate products that some primitives create?

I don't actually have a solution. We don't want to add this to astrodata since we plan to split it out. Having recipe_system control it makes sense but I don't know when to add it.

Let's think ahead...

The versions that matter are the version of recipe_system (a bit) but mostly the version of geminidr. Think of the use where a third-party package, say maroonxdr, is used. We need that version number too. It will use geminidr and maroonxdr, and "dragons". geminidr can be DRGNSVER, I don't see splitting geminidr out of DRAGONS anytime soon. Yet, I'd like to have a mechanism that would allow the external packages to add their version number too.
In the prepare primitive? Some setVersion primitive? I'd like to get Chris' opinion on this.

@KathleenLabrie
Copy link
Contributor

Another worry I have is what happens with this PR implementation when I do reduce -r display which does not change the data and therefore does not write an output. I should not. Will adding that header by default suddenly "change the file" and then force an output to be written?

@olyoberdorf
Copy link
Contributor Author

writeOutputs would work as well, but I really defer to you and Chris where this would best live. In the _write_final, it is only saving the file if the filename changed, or a suitable suffix has been set.

@codecov
Copy link

codecov bot commented Jan 19, 2022

Codecov Report

Merging #299 (dbcd7b0) into master (fa6cf30) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #299   +/-   ##
=======================================
  Coverage   57.88%   57.89%           
=======================================
  Files         249      249           
  Lines       28094    28097    +3     
=======================================
+ Hits        16263    16266    +3     
  Misses      11831    11831           
Flag Coverage Δ
gmosls 39.21% <100.00%> (+<0.01%) ⬆️
integration 34.23% <100.00%> (+<0.01%) ⬆️
regression 29.95% <0.00%> (-0.01%) ⬇️
slow 44.05% <100.00%> (+<0.01%) ⬆️
unit 47.85% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
recipe_system/reduction/coreReduce.py 69.01% <100.00%> (+0.44%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fa6cf30...dbcd7b0. Read the comment docs.

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.

2 participants