Skip to content
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

[WIP] Release v0.4.0 #134

Merged
merged 62 commits into from
Feb 5, 2019
Merged

[WIP] Release v0.4.0 #134

merged 62 commits into from
Feb 5, 2019

Conversation

bhancock8
Copy link
Contributor

@bhancock8 bhancock8 commented Jan 25, 2019

This PR gathers a series of issue fixes and upgrades. The largest breaking changes will be the upgrade to pytorch 1.0.0, restructuring to move all but the default input module (IdentityModule) into contrib/, and changing the structure of the logger/checkpointer/tensorboard config (more details to come on what that now looks like).

@bhancock8
Copy link
Contributor Author

Fixes #132

@bhancock8
Copy link
Contributor Author

See the "Logging" section under the metal_commandments for an overview of the logging changes.

Copy link
Contributor

@ajratner ajratner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@@ -3,6 +3,6 @@ multi_line_output=3
include_trailing_comma=True
force_grid_wrap=0
combine_as_imports=True
line_length=80
line_length=88
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm seeing more libraries default to 88 instead of 80 (including our code formatter black, for which 88 is the default). It just makes the code a lot more readable; a whole lot of lines don't need to wrap now with just those extra 8 characters.

- numpy
- pandas
- pytorch=0.4.1
- pytorch=1.0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Woohoo!


def score(
# TEMPORARY FOR SLICING PROJECT; there may be a better way to do this
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be included on master? Probably not?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope! Good catch. Removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants