CORE-GATECH-GROUP / serpent-tools

A suite of parsers designed to make interacting with SERPENT output files simple and flawless
http://serpent-tools.readthedocs.io/en/latest/
MIT License
52 stars 33 forks source link

ENH Deprecate settings interface #339

Open drewejohnson opened 5 years ago

drewejohnson commented 5 years ago

I've been mulling something over for the past few months, and I think we can and should completely remove the settings interface. Instead, all of the functionality can be passed to the readers at construction.

Most of readers just pull from the rc global to create attributes during the construction. Instead of

import serpentTools
serpentTools.settings.rc["depletion.materialVariables"] = ["ADENS", "MDENS"]
serpentTools.settings.rc["depletion.materials"] = ["fuel*"]
dep = serpentTools.read("my_dep.m")

one would instead use

import serpentTools
dep = serpentTools.read("my_dep.m", materials=["fuel*"], variables=["ADENS", "MDENS"])

The benefits are

  1. Remove a lot of code and tests [temporary settings using rc as a context manager]
  2. Cleaner API
  3. Changes can be made in isolation, i.e. if we want to add new options to a specific reader, we don't have to update settings, the reader, and the settings documentation. We just update the reader

The rc object and the capabilities allowed are a powerful feature of serpentTools IMHO. So naturally we will want to preserve this control, but try and reduce the pain of this switch.

Impact

Anyone who uses the rc object will be impacted. I frequently use these settings to control what data is read from depletion and result files.

Roll out

Version 0.9.0

drewejohnson commented 4 years ago

When the settings interface is removed and/or deprecated, the command line interface subcommand list should be adjusted and/or removed https://github.com/CORE-GATECH-GROUP/serpent-tools/blob/d6b65c8d5dc605fe5ca429e75047eb2b7e56530c/serpentTools/__main__.py#L159-L171

drewejohnson commented 4 months ago

Tentative plan from #516

drewejohnson commented 3 months ago

For a deprecation, it's technically API compatible. But the full removal will be not compatible