Requirements for SciKeras v0.2.0 about scikeras HOT 15 CLOSED

Thank you for putting this together!!

I think we should include re-writing docs right?

As far as features, I'd like to see:

• class_weights param. I have a hunch this may be more complicated than it seems (#52)
• epochs param. I think this may require some discussion of following the scikit-learn single param convention or using what keras uses (initial/stop) (#51)
• classes param for partial fit (#69). This can either be implemented or deemed not possible, either way.

from scikeras.

• #52, which would provide a class_weights parameter
• #51 provide an epochs parameter
• #69, classes param for partial_fit.

@stsievert I think we can probably leave these for a future release, which I think leaves us just with documentation, but do we want to also think about the data conversion interface for this release? It would probably require some type of API change (only to the "new" APIs, not the "TF" APIs). I made #83 to track this. I feel that it may heavily influence how we address the loose ends in #79.

from scikeras.

I think we can probably leave these for a future release, which I think leaves us just with documentation

I think documentation is the most pressing issue right now (I'm inclined to think it's blocking release). I don't think #51 or #52 rise to that level, though they'd be nice to have.

from scikeras.

Agreed. #51 and #52 are certainly features and not bugs/API problems, so I think it's okay to leave them out. I will work on the docs over the next week or so.

Do you think we need to clean up data conversion handling (#83) before a release? There is at least one bug (#78) related to this, and I think this API may cross over with the compile API (in terms of the loss function affecting data reshaping).

Should we address some/all of the tasks in #79, or can we ship the compiling feature as is after #66?

from scikeras.

Looping back to the open tasks here (aside from documentation).

• #52, which would provide a class_weights parameter
• #69, classes param for partial_fit.

I think these two should actually be straightforward after #88, and I've opened #103 and #104 respectively. They also add new use cases that can not easily be implemented on the user side.

I'm going to edit the OP to reflect that we have these two + documentation as the only remaining issues before a release.

from scikeras.

I'm going to edit the OP to reflect that we have these two + documentation as the only remaining issues before a release.

@adriangb could you edit the original post to put the original content back in? I'd rather not lose those notes, and I think they'll make a good release note summary.

from scikeras.

Sure, I'll re-organize them instead. We have largely the same info in the v0.2.0 project you created I think.

Edit: done. Let me know if that works.

from scikeras.

We have largely the same info in the v0.2.0 project you created I think.

Not quite. I quickly abandoned that project because keeping notes in an issue is easier. One recent example: I added a note on a specific piece of documentation to include in #88 (comment). I remember adding some other notes in other issues or PRs too, and linking to other issues (e.g., I think I mentioned #100).

In the future, I would greatly appreciate asking permission before deleting content I create, whether it be deleted by comment editing (e.g, this issue) or removing commits (i.e force-pushing). To my knowledge, no content has been lost due to force-pushing but it makes it harder for my to collaborate with you (I think in #93 I had to delete my local branch and recreate it).

from scikeras.

Hey I hear you. I try to avoid force pushing to a great extent, and typically ask for permission before making changes to someone else's work (eg. in #93 (comment)) especially since there are there are now regular collaborators in this repository (namely you).

I was editing this issue as well as the project, I thought it was duplicate information to some extent and was trying to simplify tasks to make things clearer for posterity; I'm sorry if I muddled things and made them harder.

Regarding the specific content of this OP, I restored it from your last edit, I do not see any mention of #100 in the edit history, but I am glad to add it.

In the future, I will refrain from editing any of your posts, unless expressly discussed on an edit-by-edit basis.

Finally, should we close the v0.2.0 project if it is not useful?

from scikeras.

I restored it from your last edit

Thanks! That's news to me; I wasn't aware GitHub had that feature. That significantly reduces my concern.

should we close the v0.2.0 project if it is not useful?

Yea, I think we should. I don't look at it and it's more useful to have this channel of conversation.

I try to avoid force pushing to a great extent

When do you need to force push? When the server rejects my commits, I run git pull, resolve any merge conflicts, then run git push.

from scikeras.

Yea, I think we should.

👍

When do you need to force push? When the server rejects my commits, I run git pull, resolve any merge conflicts, then run git push.

Almost never. I generally only ever do it when I end up with diverging branches and would rather start from scratch based on a diff than rebase and resolve conflicts (as I suspect may happen with #104 and #103 after #88 gets merged). I suspect in all cases some git kong fu would solve the issue.

from scikeras.

would rather start from scratch based on a diff than rebase and resolve conflicts

Yeah, rebasing is not fun. I used to do it rather frequently, but try to avoid it now. I try to resolve merge conflicts instead of rebasing. Let's say #88 gets merged, and #104 and #88 are diverging:

(master) $ git pull  # pull changes from #88 (or `git pull upstream master` on a fork)
(master) $ git checkout classes-param
(classes-param) $ git merge master
# resolve merge conflict
(classes-param) $ git commit
# merge commit created
(classes-param) $ git push

Now #104 will show a merge commit and only show the relevant changes.

from scikeras.

I'll give it a shot again this time around. Maybe I'll learn some new tricks along the way.

from scikeras.

Final release notes:
Completed as part of this release:

• Move data transformations to a Scikit-Learn Transformer based interface (#88)
• Add Keras parameters to BaseWrapper.init (loss, optimizer, etc) (#47, #55)
• Remove needless checks/array creation (#63, #59)
• Make pre/post processing functions public (#42)
• Some stability around BaseWrapper.__call__ (#35)
• Cleanup around loss names (#38, #35)
• Parameter routing (#67)
• Rename build_fn to model with deprecation cycle (#98)
• Add ability for SciKeras to compile models (#66)
• class_weights parameter (#52, #103)
• classes param for partial_fit (#69, #104)
• Provide an epochs parameter (#51, #114)
• Updated docs, now hosted on RTD (#58, #73)
• Checks to make sure models are correctly compiled (#86, #100, #88)

Scheduled for future work:

• Add more tests using sklearn classifiers/regressors that check output shapes and dtypes for different types of target types and target dtypes. Similar to the current test_multilabel_classification test (#119)
• #66: globbing / param groups for routed params (#118)

from scikeras.

Closing this issue since v0.2.0 is released.

from scikeras.