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

Refactor sample_with_mcmc class #141

Open
7 tasks
cpaniaguam opened this issue Mar 1, 2024 · 0 comments
Open
7 tasks

Refactor sample_with_mcmc class #141

cpaniaguam opened this issue Mar 1, 2024 · 0 comments

Comments

@cpaniaguam
Copy link
Member

  • Class Name: The class name sample_with_mcmc is not PEP 8 compliant. According to PEP 8, class names should use the CapWords (or CamelCase) convention. A more appropriate name would be SampleWithMCMC.

  • Library as Class Attribute: The LM object, which is an alias for the lmfit library, is being accessed as a global variable and attached to the class instance with self.LM = LM. This is unusual and can lead to confusion. It's generally better to directly use the library or module in the class methods, without attaching it to self. If the goal is to allow for different optimization libraries to be used, a better approach might be to pass in the specific functions needed as parameters, or to use a strategy pattern to allow for different optimization strategies.

  • Documentation: The docstring includes example usage, which is good, but it doesn't elaborate on what its methods do. Furher, the class and its methods lack comments explaining what they do. Adding comments would make the code easier to understand and maintain.

  • Method Names: The method plot_optimze seems to have a typo in its name. It should probably be plot_optimize.

  • Verbose Flag: Use a logger to manage calls to print to report ouput.

  • Method Complexity: Some methods like sample, optimize, and brute are doing multiple things (setting up parameters, calling minimize, printing reports). Consider decoupling them down into smaller methods, each with a single responsibility.

  • Plotting Inside Class: The class methods plot_sample and plot_optimize are directly creating plots. This might limit the flexibility for users who want to customize plots or use a different plotting library. Consider returning the data needed for plotting and let the users create their own plots.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant