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

Basis Sets Tutorial Notebook #145

Closed
wants to merge 9 commits into from

Conversation

maximilianvz
Copy link
Collaborator

Steps

  • Write a good description of what the PR does.
  • Add tests for each unit of code added (e.g. function, class)
  • Update documentation
  • Squash commits that can be grouped together
  • Rebase onto master

Description

This is a separated notebook corresponding to the Basis Sets section of the notebook from PR #142 (closed).

Type of Changes

Type
🐛 Bug fix
✨ New feature
🔨 Refactoring
📜 Docs

Related Issue

@FarnazH FarnazH requested review from marco-2023 and FarnazH January 10, 2024 19:14
@marco-2023
Copy link
Contributor

Hi @maximilianvz @FarnazH I don't have a way to push commits to this PR. I am pushing some changes to https://github.com/marco-2023/gbasis/tree/basis_set_tutorial. Max, when I am finished you can pull them to your local branch and then push them to this PR. I think the examples are great but need to show more gbasis features. I am planning to finish polishing this Notebook and we can use it as a sort of template for the others.

@leila-pujal
Copy link
Collaborator

hi @marco-2023, I think you should be able to push changes to Max's PR. I think you have to add Max's repo and fetch his branch (I use this https://git-scm.com/book/en/v2/Git-Basics-Working-with-Remotes). Let me know if you need help trying it.

@marco-2023 marco-2023 mentioned this pull request Jan 12, 2024
5 tasks
@marco-2023
Copy link
Contributor

marco-2023 commented Jan 12, 2024

Hi @maximilianvz @FarnazH I made a new PR #158 (it is the finished version of this one but was rebased on the master branch). I thought it would be better than a force push here. I think it may be better if we continue making the necessary revisions there. I will continue going through the evaluations tutorial.

@maximilianvz
Copy link
Collaborator Author

@FarnazH and @marco-2023, if we can agree to move ahead with PR #158, we should close this PR. @marco-2023, do you think you'll have to take this approach (rebasing and then making a new PR) for the other two notebooks as well?

@marco-2023
Copy link
Contributor

Hi @maximilianvz for the other notebooks we need the changes that were made to the API merged in the branches in which we are making the notebooks. The options I see for doing this are:

  1. merge master into each of the notebook branches
  2. rebase the notebook branches into master and force-push the branches
  3. rebase the notebook branches into master and push them as new branches/PRs

I am in favor of either 2 or 3 (the approach seems cleaner), 3 is the more conservative approach. If you could 2 on the other two notebook branches it would be perfect.

@maximilianvz
Copy link
Collaborator Author

@marco-2023, I just finished doing the second approach you suggested on the other 2 notebooks. If you still wish to go with the third option, I'll leave that up to your discretion.

@marco-2023
Copy link
Contributor

Perfect @maximilianvz, I will add my updates to the current branches then. Thank you very much.

@FarnazH
Copy link
Member

FarnazH commented Jan 12, 2024

See #158.

@FarnazH FarnazH closed this Jan 12, 2024
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