-
Notifications
You must be signed in to change notification settings - Fork 0
Precommit #64
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
Precommit #64
Conversation
Allow remove-tables --all Fixed sampled choices Removed surplus df.py stuff
stefpiatek
left a comment
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.
Very much rubber stamping with a comment
|
|
||
|
|
||
| @lru_cache(1) | ||
| def everything_factory() -> GeneratorFactory: |
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.
ooh nice, is there a reason that this is better than making an object here, that's imported? I presume you may want to edit the output?
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.
No, could just make an object. The reason I did it like this was to try to make startup a little shorter. Startup still takes too long, though and I'm not sure this made any noticeable difference. I could do some benchmarking...
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.
Ah yep startup makes sense. may as well only run it when you need it 😄. Possibly worth commenting this is why you've done it?
Finally! All the pre-commit hooks are passing, and all the tests are running on github and passing.
This was a huge job!