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

Fix data save issue for non-square patterns #486

Merged
merged 4 commits into from
Dec 12, 2021
Merged

Conversation

IMBalENce
Copy link
Contributor

This is quick patch for fixing data save issue mentioned in #485

Tested on h5ebsd file saved from Oxford .ebsp data file. The saved pattern dimensions are consistent with original patterns.

@hakonanes
Copy link
Member

Thanks! The failing test is due to #484, I'll include a fix for this in this PR, hope that's OK. I'll also add a test for saving and loading non-square patterns.

Could you point this to main instead of develop? This should be released right away, and so should go to main.

@hakonanes hakonanes added bug Something isn't working tests This relates to the tests labels Dec 11, 2021
@hakonanes hakonanes added this to the v0.5.5 milestone Dec 11, 2021
@IMBalENce IMBalENce changed the base branch from develop to main December 11, 2021 13:08
@IMBalENce
Copy link
Contributor Author

Thought I have selected main. Just corrected it. Thank you.

@hakonanes
Copy link
Member

Ah, we don't want all these extra commits. Can you either (1) rebase your commit on the latest commit in main or (2) branch out from the latest commit on main and do the change there?

@IMBalENce
Copy link
Contributor Author

Sorry for that. Just realise what happened after some reading. I hope this has been fixed now.

@hakonanes
Copy link
Member

Seems to have done the trick! Did you rebase and then force push? I've used git for a while, but am no expert, so I'm curious.

Signed-off-by: Håkon Wiik Ånes <hwaanes@gmail.com>
Signed-off-by: Håkon Wiik Ånes <hwaanes@gmail.com>
Signed-off-by: Håkon Wiik Ånes <hwaanes@gmail.com>
@hakonanes
Copy link
Member

hakonanes commented Dec 12, 2021

I'll merge after checks pass.

FYI: This merge contains a commit with a change in the release version (0.5.4 -> 0.5.5), so a tagged release draft will be automatically made (script run from GitHub Action). I'll touch up the draft if necessary (most likely not), and then publish. This publishing will trigger this action which will build the source distribution and upload it to PyPI! Easy peasy.

@IMBalENce
Copy link
Contributor Author

Seems to have done the trick! Did you rebase and then force push? I've used git for a while, but am no expert, so I'm curious.

Yes, I rebased and pushed the normal way, and got rejected. Luckily, There is a stackoverflow post about this, so I tried force push. Looks like it works.

I also tried created a new branch from main and made the change. But I'm not sure if I create a PR from the new branch, it would duplicate the current PR.

FYI: This merge contains a commit with a change in the release version (0.5.4 -> 0.5.5), so a tagged release draft will be automatically made (script run from GitHub Action). I'll touch up the draft if necessary (most likely not), and then publish. This publishing will trigger this action which will build the source distribution and upload it to PyPI! Easy peasy.

Very cool. Thanks for the info. I'll take a read to see how it works.

@hakonanes
Copy link
Member

I also tried created a new branch from main and made the change. But I'm not sure if I create a PR from the new branch, it would duplicate the current PR.

No problem creating a new PR and closing this one, the important thing is the git history, not the GitHub "history" with open/closed PRs, issues etc. In this case I would have (1) pulled the last changes in main locally, (2) branched off of main, (3) done the change, (4) make PR.

Very cool. Thanks for the info. I'll take a read to see how it works.

Sure, I think it is very cool indeed (: Two branches are more complicated than one branch, but having this and automation makes it a lot simpler to release patch releases in my opinion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working tests This relates to the tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants