CORE-GATECH-GROUP / serpent-tools

A suite of parsers designed to make interacting with SERPENT output files simple and flawless
http://serpent-tools.readthedocs.io/en/latest/
MIT License
52 stars 33 forks source link

[BUG] Fix of MicroXSReader precheck mechanics #465

Closed aalfonsi closed 1 year ago

aalfonsi commented 2 years ago

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

  1. [DOC] if documentation related
  2. [DEP] if something is deprecated or a deprecated feature is removed
  3. [ENH] if a feature or enhancement
  4. [BUG] if a bug fix
  5. [API] if a (possibly incompatible) change to the API
  6. [TST] if related to our test suite
  7. [STY] if related to code cleanliness and pythonicness

For bug fixes and enhancements, link to the corresponding issue.

Closes https://github.com/CORE-GATECH-GROUP/serpent-tools/issues/464

Make sure you have read over the developer guide to ease the review process. These include:

Include a thorough and concise overview of the changes, and why this PR is overall beneficial to the project.

Once the PR has been created and is nearing closure, make sure that serious changes, including deprecations and incompatible changes are documented in the changelog

DanKotlyar commented 2 years ago

Thanks @aalfonsi @drewejohnson - should I take any action?

drewejohnson commented 1 year ago

@DanKotlyar I'm not sure why py 3.9 isn't going in testing. But once that gets resolved you should be able to merge this in

drewejohnson commented 1 year ago

@aalfonsi can you change the source branch from develop to master? We don't have py3.9 testing on develop -

https://github.com/CORE-GATECH-GROUP/serpent-tools/blob/f61cd104bd997aa80429f1059bbc3669adc18654/.github/workflows/testing.yaml#L20

aalfonsi commented 1 year ago

Unfortunately if I do, this PR will add ~90 files with several conflicts:

Screen Shot 2022-10-14 at 12 22 18 PM

Probably the best solution (if master is the real "default" branch) is for me to apply these changes in a new branch from master. What do you think?

aalfonsi commented 1 year ago

Unfortunately if I do, this PR will add ~90 files with several conflicts:

Screen Shot 2022-10-14 at 12 22 18 PM

Probably the best solution (if master is the real "default" branch) is for me to apply these changes in a new branch from master. What do you think?

Oh never mind...you said it :) "source branch" not "target branch". No problem...I can do it.

aalfonsi commented 1 year ago

microxs_patch.txt

Since the "master" branch is neither a protected branch or a default branch, it does not show up in my fork. The only way to contribute to master for me is to create a patch file (as attached).

DanKotlyar commented 1 year ago

@drewejohnson How should I proceed with this?

aalfonsi commented 1 year ago

@drewejohnson How should I proceed with this?

Since the modification is quite small, Can one of you apply the patch in a new branch (based on master) and then we can close this MR ?

drewejohnson commented 1 year ago

@aalfonsi sorry for the confusion. I'll take your patch and create a new PR off master

@DanKotlyar can you take a look at these comments - https://github.com/CORE-GATECH-GROUP/serpent-tools/issues/466#issuecomment-1279286356 This should help resolve some confusion going forward