BrainLesion / eReg

TODO
Apache License 2.0
1 stars 0 forks source link

Added a whole bunch of default values #59

Closed sarthakpati closed 7 months ago

sarthakpati commented 7 months ago

I am pretty sure there is a better way to do this (for example, having a base Config class under ereg.configurations that reads the sample_config.yaml and populates a baseline Config object, which can then be used in the RegistrationClass class.

However, I unfortunately don't have the kind of time on hand right now to make this change. I would really appreciate it if someone could help me put this together.

Includes #57, so that should/will get merged along with this.

brainless-bot[bot] commented 7 months ago

🤖 Code Formatting Reminder

Hello there! 👋 It looks like the code in this pull request might benefit from some formatting improvements. Fix the issues locally or use our auto format action by commenting /format on this PR!

Code style: black

neuronflow commented 7 months ago

/format

brainless-bot[bot] commented 7 months ago

:robot: I will now format your code with black. Check the status here.

neuronflow commented 7 months ago

I am pretty sure there is a better way to do this (for example, having a base Config class under ereg.configurations that reads the sample_config.yaml and populates a baseline Config object, which can then be used in the RegistrationClass class.

However, I unfortunately don't have the kind of time on hand right now to make this change. I would really appreciate it if someone could help me put this together.

can you open a feature request so we don't lose track of this? @sarthakpati

neuronflow commented 7 months ago

@sarthakpati currently, unit tests are not passing yet.

neuronflow commented 7 months ago

once this passes unit tests and is merged, we can merge https://github.com/BrainLesion/eReg/pull/57 to require passed tests before merging, so we don't break ereg again :)

sarthakpati commented 7 months ago

How do you feel about putting default values in a yaml file to create a single source of truth?

Sure, but let's tie it in with #60. Doing yaml I/O in a purely computational class is just clunky.

brainless-bot[bot] commented 7 months ago

🤖 Code Formatting Reminder

Hello there! 👋 It looks like the code in this pull request might benefit from some formatting improvements. Fix the issues locally or use our auto format action by commenting /format on this PR!

Code style: black