Skip to content

Comments

vignetmaker Clean Up#411

Merged
martinkilbinger merged 2 commits intoCosmoStat:developfrom
sfarrens:vignetmaker_cleanup
Jul 15, 2021
Merged

vignetmaker Clean Up#411
martinkilbinger merged 2 commits intoCosmoStat:developfrom
sfarrens:vignetmaker_cleanup

Conversation

@sfarrens
Copy link
Member

@sfarrens sfarrens added this to the First Code Clean Up milestone Jun 23, 2021
@sfarrens sfarrens self-assigned this Jun 23, 2021
@sfarrens sfarrens mentioned this pull request Jun 23, 2021
13 tasks
@sfarrens
Copy link
Member Author

@aguinot I could only see some very minor difference between vignetmaker_runner.py and vignetmaker_runner2.py so I have based the first pass of the refactoring on vignetmaker_runner2.py, which includes sqlitedict. Is there any need to have both?

When you have time, please also send me some example input data (with appropriate config file options) I can use to test this module.

@aguinot
Copy link
Contributor

aguinot commented Jun 29, 2021

I created a config file to test both way to run the vignettes_maker on CANDIDE at: /home/guinot/config_test_VM.ini
You will need to runner as you will see. I left the OUTPUT_DIR empty so you will have to pick one.
If you want to compare the output you will find them at /automnt/n17data/guinot/simu_W3/pipeline_output/run_sp_MaSxPsViSmVi_2021-03-23_18-19-08/
Let me know if encounter an issue.

Copy link
Contributor

@martinkilbinger martinkilbinger left a comment

Choose a reason for hiding this comment

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

Looks good to me.

We should discuss together:

  • removal of vignetmaker_runner2. I am in favor of this, however: the reason some modules have a *RUNNER2 version is multiple calls to the same module in one config file. One solution could be to split the config file into two parts. And to assign unique run names with which subsequent module calls can refer those runs. Will require a bit of work.
  • Naming convention, some previous module have capitalized package names, do we want to make this consistent?

@martinkilbinger martinkilbinger merged commit 6b0ebc4 into CosmoStat:develop Jul 15, 2021
@martinkilbinger martinkilbinger deleted the vignetmaker_cleanup branch July 15, 2021 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Management] Module clean up: vignetmaker_runner and vignetmaker_runner2

3 participants