Enable context for SMAC Optimizer #741
Conversation
mypy fixups
| return f"{self.name}({opt_targets},config={self._config})" | ||
|
|
||
| def __enter__(self) -> 'Optimizer': | ||
| def __enter__(self) -> "Optimizer": |
There was a problem hiding this comment.
There's a whole bunch of style-only changes in here that makes reviewing a little difficult.
Can you please revert those, or separate them out to a different PR?
@ agree |
|
I think we agreed to stall this on until #744 is done to make these changes easier to consume. |
| self._in_context = False | ||
|
|
||
| experiment_id = self._global_config.get('experiment_id') | ||
| experiment_id = self._global_config.get("experiment_id") |
There was a problem hiding this comment.
here and everywhere: let's separate cosmetic fixes from functional changes. mixing the two bloats up the diff and makes reviewing PRs much harder. Please keep the essential functionality in this PR and make the diff as small as possible and move all code annotations, style, and docstring improvements to a separate PR. having more PRs is good for your github karma! 😄
There was a problem hiding this comment.
Sounds good! #744 should do just the style changes
There was a problem hiding this comment.
Yeah, @motus @jsfreischuetz and I already talked about that. #744 is the start of that, but I think will also need some additional work. #746 unearthed some related topics around pyproject.toml settings vs. setup.cfg for using black and how setup and build type dependencies are specified (right now we require conda for certain ones instead of pip, which as we saw, has some issues).
Am currently working on improvements for all of those and will send a split out series of PRs soon for:
- pyproject.toml settings and build related improvements
- annoyed at this one since there's no "include" feature, so it means duplicating some content across files it looks like
- adding
blackandisortMakefilerules,vscodesettings, configs, etc.- that's a part of this one, but I'm going to restrict it to a smaller set so it's easier to see the changes
- requiring them in the build (including all of the reformatting of all files)
- this will be a larger change, so I want make sure it's just reformatting. Can take that up here or just create a new PR
- adding the revision from 3 in the
.gitrevisionslist to ignore it from mostgit blametypes of analysis
|
Replaced with #751 |
3 participants
No description provided.