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

[ENH] Sensitivity DataFrame #483

Closed
wants to merge 4 commits into from
Closed

Conversation

GrimFe
Copy link

@GrimFe GrimFe commented May 9, 2023

Please add the appropriate tag to the beginning of the title of your PR:

The toDataFrame method was added to the SensitivityReader class.
This allows for easy energy dependent sensitivity manipulation in a pandas context and follows what was implemented for the DepletedMaterial class.

Grimaldi Federico and others added 4 commits May 9, 2023 09:34
@drewejohnson
Copy link
Collaborator

@GrimFe thanks for this contribution! I think this will be a good addition.

We're in the process of moving the primary development branch from develop to "main" (#474 // #466 (comment)). For this PR, that means the branch this PR should be merging into should be main and not develop. That should trigger testing, instead of showing lots of tests waiting to be reported.

Can you change the destination branch to be main? Or close this PR and open a new PR merging these changes into main? There could be some issues w/ the branch histories diverging between develop and main, so these commands should work

git switch -c new-sens-dataframe main
git cherry-pick 991820e8f78eeded0e1cf4c5ff021acc11141323 3a50d1429fb9b4e978000b7922c035a307ab7f2c 6df2b83328a43235ce6ae145766e69e7db8e6c44

which will create a branch called new-sens-dataframe (or whatever you want) starting from the main branch. The three commits in the cherry-pick are the commits from this PR. There is a small conflict in the contributors file which is easily resolvable

diff --cc docs/contributors.rst
index f3647cc,d7b4c17..0000000
--- a/docs/contributors.rst
+++ b/docs/contributors.rst
@@@ -13,4 -13,4 +13,8 @@@ Here are all the wonderful people that 
  * `Paul Romano <https://github.com/paulromano>`_
  * `Anton Travleev <https://github.com/travleev>`_
  * `@nicolaborate <https://github.com/nicoloabrate>`_ 
++<<<<<<< HEAD
 +* `@rzehumat <https://github.com/rzehumat>`_ 
++=======
+ * `Federico Grimaldi <https://github.com/GrimFe>`_ 
++>>>>>>> 6df2b83 (Update contributors.rst)

I'm working on getting our testing to work with newer versions of python (we try to test against 35 and 36) in #478, so these changes still might make a situation where tests will run but some won't pass. But that'll be my problem to fix 😁

@drewejohnson drewejohnson self-requested a review May 22, 2023 15:05
@GrimFe
Copy link
Author

GrimFe commented Jun 26, 2023

@drewejohnson thanks for your reply and sorry for my (very long) absence.

I just tried to pull request from my develop branch to yours. Now that I did it it feels rather wrong (I am kinda new to these things so most likely it is). Maybe I should just pull from your develop first, is this the case?

In parallel I am also trying what you suggested with the new-sen-dataframe branch on my local repository.
Again, I am not fully aware of what the commands you suggest do, but my understanding is that git switch -c will create a new branch named new-sens-dataframe from the main branch. Then I will pick some specific commits to add. Is this right?
If that is the case, I have two questions:

  • does it matter the branch I do the switch from? (I run all the commands from my SensitivityDataframe branch so far)
  • I get an error: fatal: bad object 6df2b83328a43235ce6ae145766e69e7db8e6c44 is this something crucial to the procedure? (running git log in my local SensitivityDataframe branch I cannot find that commit, if this piece of information is relevant)

Thank you very much for your support

@drewejohnson
Copy link
Collaborator

@GrimFe No worries. I wouldn't be in a place to hold bad feelings since I've taken so long to get back to this and #486 (which I'll get to in a bit)

I take full responsibility for this situation we've found ourselves in. It was not communicated that we were shifting away from the develop branch in our contribution guidelines.

Making comments about this and #486 at the same time. I propose the following

I propose the following

  1. We close this PR
  2. We close [ENH] Sensitivity DataFrame #486
  3. I will put forward a PR with your changes from this PR but off the main branch to help highlight just your changes
  4. I will send you a note for the review to make sure the changes are indeed what you envisioned.

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.

2 participants