idaholab / MontePy

MontePy is a Python library (API) to read, edit, and write MCNP input files.
https://www.montepy.org/
MIT License
25 stars 5 forks source link

Idea: Formatter class #103

Open MicahGale opened 1 year ago

MicahGale commented 1 year ago

In GitLab by @tjlaboss on Aug 26, 2022, 13:02

Several Python libraries, such as logging, allow users to pass Formatter objects.

This could be useful for controlling MontePy outputs. Attributes might be:

The syntax could be either MCNP_Problem.formatter = Formatter(...) or MCNP_Problem.write_to_file(..., Formatter(...)). Basically, MontePy would internally pass around a Formatter instead of mcnp_version.

MicahGale commented 1 year ago

Will need some significant rework for now.

MicahGale commented 1 year ago

This does feel a bit design patterny. Is there a better way to do this? I'm just hesitant with any design patterns.

MicahGale commented 1 year ago

In GitLab by @tjlaboss on Sep 6, 2022, 15:56

A few alternatives, off the top of my head:

The thing I don't like about the keyword arguments approach that it is messy to implement piecemeal. Tends to lead to a lot of shotgun surgery. Often violates DRY when using default arguments. Using a **kwargs approach alleviates some of this pain, especially when used with .get(key, default).

Having them directly as attributes of MCNP_Problem just doesn't feel right. That object is normally instantiated by reading an existing file.

The Formatter and the options options are variations on the same theme and will "feel" basically the same.

MicahGale commented 1 year ago

In GitLab by @tjlaboss on Sep 6, 2022, 16:04

Basically the advantage of having them as attributes (or a dict with kwargs.get()) is that it will be much neater to add future formatting configuration.

Passing a configuration object--be it a dict, Formatter, or mcnp_version tuple--feels clunky. Using one of the options configurations gets rid of that, but the global or context variable approach is less obvious.

MicahGale commented 1 year ago

Yep fair. I think we should have the problem have a formatter object. You can override this with passing one to the write_to_file but I think having it attached to the problem so you have time to set the options would be best.

MicahGale commented 1 year ago

@tjlaboss probably should look at the infrastructure I'm creating for !58. This maybe the best place to integrate it. I'm thinking specficially in ValueNode: https://hpcgitlab.hpc.inl.gov/experiment_analysis/mcnpy/-/blob/48-add-support-for-keeping-comments/mcnpy/input_parser/syntax_node.py#L154