Warn about unused indexing.refinement_protocol params
#2586
Conversation
|
This rabbit hole goes deeper than expected... Only now have I realized that |
|
I have determined that |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2586 +/- ##
==========================================
+ Coverage 78.54% 78.59% +0.05%
==========================================
Files 609 609
Lines 75054 75319 +265
Branches 10712 10809 +97
==========================================
+ Hits 58949 59195 +246
- Misses 13940 13950 +10
- Partials 2165 2174 +9 |
|
First things first, I discovered the inconsistency with logging is not an error, but rather comes from changes to phil parameters discussed in #2587. After enabling logging in Personally, I would like to merge this PR because I believe it introduces much-needed verbosity to distinguish between the refinement protocol of both indexers. The warning messages add a layer of safety, but they can be probably seen as clutter, so I would appreciate everyone's opinion on how/if this should be included. |
|
It's been 2 weeks and I don't want to leave this PR hanging while I move to work on something else right now, so I'll take the liberty to squash-merge this change into main. It only increases verbosity and should not affect other functionality. |
* Update phil, warn if setting unused params for stills indexer * Introduce style changes suggested by black * Add news fragment * Phil scope does not accept ';' in help string * Phil scope was missing '"' * Correctly access the indexing scope of the phil * `disable_unit_cell_volume_sanity_check` is used in both indexers * Don't hardcode default parameter values
* Update phil, warn if setting unused params for stills indexer * Introduce style changes suggested by black * Add news fragment * Phil scope does not accept ';' in help string * Phil scope was missing '"' * Correctly access the indexing scope of the phil * `disable_unit_cell_volume_sanity_check` is used in both indexers * Don't hardcode default parameter values
Some phil parameters remain unused when provided to
StillsIndexer. Since there are no imminent plans to expand the capabilities of this indexer, this PR suggests increasing the verbosity of indexing phil string and logging subtle warnings when unused phil parameters are being set. Resolves #2584.