-
Notifications
You must be signed in to change notification settings - Fork 38
Conversation
So they're not at the top of the project.
Sorry, lost the commit history here :(
We have to call the package sf-hamilton, since there is already a hamilton package in pypi. I moved the version into the hamilton package itself, that way it's easy to check the running version whereever it ends up being used.
Add with pre-commit install.
In python 3.6 and python 3.7.
It was writing within hamilton -- that's just unnecessary.
Vestage from old repository. Not required here.
So that things are simpler.
Looks good. A thought: we should probably include hamiltime here as well, or in a different repo. Its more closely coupled to the internals of hamilton than demandpy. I think we should do this by including it as a plugin, E.G. an optional feature (installable target) that can be used with hamilton but is not required. I think we should do this with the initial release, because: Happy to do it as a plugin structure as its really extra functionality on top of hamilton. |
@elijahbenizzy sounds good. but 🤔 do we want commit history for hamiltime? If so I'll need to redo everything I think (perhaps I can google it...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@elijahbenizzy sounds good. but 🤔 do we want commit history for hamiltime? If so I'll need to redo everything I think (perhaps I can google it...)
I don't think we need it TBH. In fact, I'd go ahead and say that if we ever open source this we'll want to completely remove commit history, so I'm not too stressed about it. That said, it would be nice to git blame
, but most definitely not essential.
|
||
from setuptools import setup, find_packages | ||
|
||
# don't fail if there are problems with the readme (happens within circleci) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh? Does circleCI just drop the README?
version=VERSION, | ||
description='Hamilton, the micro-framework for creating dataframes.', | ||
long_description=readme, | ||
author='skrawczyk@stitchfix.com, elijah.benizzy@stitchfix.com', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: looks like author is supposed to be full name whereas the author_email is supposed to be the corresponding emails (in this case the team-email makes sense)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This creates a package to be released.
I believe I have ported things over correctly.
The pip installable will be sf-hamilton due to naming conflict in pypi.
I have tested this locally with demandpy, and unit tests pass. Once we push a version here, I'll then be able to properly create a branch and have it go through all the tests.