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

Add neptune.ai tracker #183

Merged
merged 17 commits into from Dec 2, 2020
Merged

Add neptune.ai tracker #183

merged 17 commits into from Dec 2, 2020

Conversation

cthoyt
Copy link
Member

@cthoyt cthoyt commented Dec 1, 2020

Closes #182

This PR does the following:

  • reorganizes pykeen.trackers to be a subpackage since it was getting pretty full for a single file
  • adds a tracker for neptune and an accompanying tutorial
  • enables mypy on a limited basis and makes a small change to the utils to pass
  • updates the README

For review purposes, the only code you need to look at is pykeen.trackers.neptune and the tutorial. No other code has changed - it was just reorganized.

@cthoyt cthoyt added the 🐺 Tracker Related to results trackers, like MLflow label Dec 1, 2020
@cthoyt cthoyt self-assigned this Dec 1, 2020
@cthoyt cthoyt marked this pull request as ready for review December 2, 2020 11:43
@cthoyt cthoyt requested review from migalkin, a team and mberr and removed request for a team December 2, 2020 11:43
@cthoyt
Copy link
Member Author

cthoyt commented Dec 2, 2020

@PyKEEN-bot test plz

@lvermue
Copy link
Member

lvermue commented Dec 2, 2020

@PyKEEN-bot Can you repeat those tests?

@lvermue
Copy link
Member

lvermue commented Dec 2, 2020

@PyKEEN-bot Can you repeat those tests?

@cthoyt
Copy link
Member Author

cthoyt commented Dec 2, 2020

@PyKEEN-bot one more time!!... test

@cthoyt
Copy link
Member Author

cthoyt commented Dec 2, 2020

@migalkin thanks for taking a look. I had a few other ideas for trackers, some inspired by the list on PyTorch Lightning (https://github.com/PyTorchLightning/pytorch-lightning/tree/master/pytorch_lightning/loggers). Do you know any other services that might be worth adding?

Copy link
Member

@mberr mberr left a comment

Choose a reason for hiding this comment

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

What happened to appveyor?

Besides I am fine with the changes.

P.S.: Four reviewers might be a bit wasteful 😅

@lvermue
Copy link
Member

lvermue commented Dec 2, 2020

What happened to appveyor?

AppVeyor doesn't want to play nicely and now we'll move to Github Actions completely

@cthoyt
Copy link
Member Author

cthoyt commented Dec 2, 2020

What happened to appveyor?

Besides I am fine with the changes.

P.S.: Four reviewers might be a bit wasteful 😅

just making sure everyone knows it's here 😅 I only needed one person to check it to make sure it didn't look crazy

@cthoyt
Copy link
Member Author

cthoyt commented Dec 2, 2020

@lvermue do we need to run CI again? or are we sometimes okay with merging even if the tests didn't run on the last commit?

@lvermue
Copy link
Member

lvermue commented Dec 2, 2020

@PyKEEN-bot Please test it one more time!

@lvermue
Copy link
Member

lvermue commented Dec 2, 2020

@cthoyt You're right. But invoking it once more will show us how it'll react now that AppVeyor is gone

@mberr
Copy link
Member

mberr commented Dec 2, 2020

@lvermue do we need to run CI again? or are we sometimes okay with merging even if the tests didn't run on the last commit?

Not sure how easy it is to implement this:

Can we enforce running the full pipeline before the actual merge happens? So once you click squash and merge, it will first run the pipeline and only execute the merge when the pipeline passes.

@migalkin
Copy link
Member

migalkin commented Dec 2, 2020

@migalkin thanks for taking a look. I had a few other ideas for trackers, some inspired by the list on PyTorch Lightning (https://github.com/PyTorchLightning/pytorch-lightning/tree/master/pytorch_lightning/loggers). Do you know any other services that might be worth adding?

I haven't used Neptune (yet), there is indeed a huge variety of such trackers. Currently, I'd say ML Flow / WANDB / Neptune is a good enough gentleman set :)

@lvermue
Copy link
Member

lvermue commented Dec 2, 2020

@lvermue do we need to run CI again? or are we sometimes okay with merging even if the tests didn't run on the last commit?

Not sure how easy it is to implement this:

Can we enforce running the full pipeline before the actual merge happens? So once you click squash and merge, it will first run the pipeline and only execute the merge when the pipeline passes.

@mberr It should be that one can only choose to merge when everything is fine. This is now enforced. Squash and Merge is only available when the relevant checks have passed and the branch is up to date.

@lvermue
Copy link
Member

lvermue commented Dec 2, 2020

@PyKEEN-bot tests ... one last time 😅

@lvermue
Copy link
Member

lvermue commented Dec 2, 2020

@lvermue do we need to run CI again? or are we sometimes okay with merging even if the tests didn't run on the last commit?

The current setup enforces that the last commit always has to be up to date and has to have passed all checks. I prefer this setup and think that it also is the golden standard to avoid any mistakes making its way to the master branch.

@lvermue lvermue merged commit 0d22383 into master Dec 2, 2020
@lvermue lvermue deleted the update-trackers branch December 2, 2020 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💎 New Component 🐺 Tracker Related to results trackers, like MLflow
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add neptune tracker
5 participants