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

Added some example notebooks #40

Merged
merged 48 commits into from
Feb 22, 2025
Merged

Added some example notebooks #40

merged 48 commits into from
Feb 22, 2025

Conversation

along4
Copy link
Collaborator

@along4 along4 commented Feb 22, 2025

This pull request involves several updates to the pleiades project, focusing on enhancing the Isotope and IsotopeParameters classes, updating the SAMMY parameter file handling, and adding logging capabilities. The most important changes include adding logging, updating the IsotopeParameters class, and modifying the SAMMY parameter file handling.

Logging Enhancements:

Class and Method Updates:

SAMMY Parameter File Handling:

Notebook Updates:

@along4 along4 requested a review from KedoKudo February 22, 2025 05:39
@along4 along4 merged commit 607a4b4 into v2.0 Feb 22, 2025
1 of 2 checks passed
@along4 along4 deleted the 39-v20-examples branch February 22, 2025 22:30
@KedoKudo
Copy link
Collaborator

@along4 Thanks for fixing the issues (and apologize for not being able to review it due to other commitments) . There are still three left as reported by pre-commit:

  • notebook/samexm/ex027/ex027.ipynb is empty
  • src/pleiades/sammy/parameters/isotope.py:153:43: ARG003 Unused class method argument: extended
    • If it is clear that extended format will never be supported, we can safely remove it from the function signature.
    • Alternatively, a warning or "not implemented" error can be used to inform the users
  • src/pleiades/sammy/parameters/radius.py:663:9: F841 Local variable where_am_i is assigned to but never used
    • it seems like a log entry for where_am_i is missing, but if this is not needed anymore, it is better to remove it.

@along4
Copy link
Collaborator Author

along4 commented Feb 23, 2025

@KedoKudo, no worries! I had time today to work out most of the bugs before the merge.

I think I am going to work on the parameters a bit more tonight and tomorrow. I would like to get an update functionality going to merge parfiles soon.

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.

3 participants