Skip to content

Conversation

@hannahbaumann
Copy link
Contributor

No description provided.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@github-actions
Copy link

github-actions bot commented Mar 7, 2024

Binder 👈 Launch a binder notebook on branch OpenFreeEnergy/ExampleNotebooks/switched_to_tyk2

@hannahbaumann hannahbaumann linked an issue Mar 7, 2024 that may be closed by this pull request
@review-notebook-app
Copy link

review-notebook-app bot commented Mar 11, 2024

View / edit / reply to this conversation on ReviewNB

IAlibay commented on 2024-03-11T10:00:02Z
----------------------------------------------------------------

[enhancement]

A Pymol render of the protein would be amazing here.


hannahbaumann commented on 2024-03-25T13:45:33Z
----------------------------------------------------------------

Added an image, if this will be our main advertising notebook, maybe Ben could make a prettier one, but maybe for right now this is also ok?

IAlibay commented on 2024-04-03T09:28:44Z
----------------------------------------------------------------

Let's go with this one for now.

@review-notebook-app
Copy link

review-notebook-app bot commented Mar 11, 2024

View / edit / reply to this conversation on ReviewNB

IAlibay commented on 2024-03-11T10:00:03Z
----------------------------------------------------------------

Wasn't this used as part of one of the tutorials at last year's OMSF workshop? It feels like a reasonably useful command - do we have an issue anywhere that aims to remove it? (or conversely add it to the help info?)


hannahbaumann commented on 2024-03-25T13:47:45Z
----------------------------------------------------------------

We should discuss this, if the plan is to use this command (it generally seems useful), we should expose it in the CLI, if we decide not to expose it there we shouldn't advertise it here, I think (you currently can't find this command with openfe --help)

IAlibay commented on 2024-04-03T09:49:29Z
----------------------------------------------------------------

I've raised an issue, it doesn't seem like removing it from help was advertised anywhere (it used to be there)

IAlibay commented on 2024-04-04T08:02:24Z
----------------------------------------------------------------

Based on the issue - let's go ahead and remove it.

hannahbaumann commented on 2024-04-11T12:00:28Z
----------------------------------------------------------------

I remove it!

@review-notebook-app
Copy link

review-notebook-app bot commented Mar 11, 2024

View / edit / reply to this conversation on ReviewNB

IAlibay commented on 2024-03-11T10:00:03Z
----------------------------------------------------------------

There's some reference to T4 lysozyme here that should be swaped to TYK2 under points 1 and 2.


hannahbaumann commented on 2024-03-25T13:48:05Z
----------------------------------------------------------------

Thanks, changed this!

@review-notebook-app
Copy link

review-notebook-app bot commented Mar 11, 2024

View / edit / reply to this conversation on ReviewNB

IAlibay commented on 2024-03-11T10:00:04Z
----------------------------------------------------------------

These probably need updating - just manually editing the metadata would be best. Otherwise folks will run the notebook and get drastically different values and wonder what's going on.


hannahbaumann commented on 2024-03-25T13:49:46Z
----------------------------------------------------------------

Removed the cell output (for now since this is just a first stab at this)

Copy link
Member

@IAlibay IAlibay left a comment

Choose a reason for hiding this comment

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

Thanks @hannahbaumann ! Here's a couple of review comments - as usual the [enhancement] tag is very much optional.

Copy link
Contributor Author

Added an image, if this will be our main advertising notebook, maybe Ben could make a prettier one, but maybe for right now this is also ok?


View entire conversation on ReviewNB

Copy link
Contributor Author

We should discuss this, if the plan is to use this command (it generally seems useful), we should expose it in the CLI, if we decide not to expose it there we shouldn't advertise it here, I think (you currently can't find this command with openfe --help)


View entire conversation on ReviewNB

Copy link
Contributor Author

Thanks, changed this!


View entire conversation on ReviewNB

Copy link
Contributor Author

Removed the cell output


View entire conversation on ReviewNB

Copy link
Member

IAlibay commented Apr 3, 2024

Let's go with this one for now.


View entire conversation on ReviewNB

Copy link
Member

IAlibay commented Apr 3, 2024

I've raised an issue, it doesn't seem like removing it from help was advertised anywhere (it used to be there)


View entire conversation on ReviewNB

Copy link
Member

IAlibay commented Apr 4, 2024

Based on the issue - let's go ahead and remove it.


View entire conversation on ReviewNB

Copy link
Contributor Author

I remove it!


View entire conversation on ReviewNB

Copy link
Member

@IAlibay IAlibay left a comment

Choose a reason for hiding this comment

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

lgtm!

@IAlibay IAlibay merged commit 73cc410 into openfe-1.0.0rc0-test Apr 11, 2024
@IAlibay IAlibay deleted the switched_to_tyk2 branch April 11, 2024 12:17
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.

Showcase: Switch to tyk2 system throughout

3 participants