Skip to content

Fix issue 4959 modifying the tests from pi to π#5199

Closed
alejomonbar wants to merge 4 commits into
Qiskit:masterfrom
alejomonbar:fixissue
Closed

Fix issue 4959 modifying the tests from pi to π#5199
alejomonbar wants to merge 4 commits into
Qiskit:masterfrom
alejomonbar:fixissue

Conversation

@alejomonbar
Copy link
Copy Markdown
Contributor

@alejomonbar alejomonbar commented Oct 8, 2020

Summary

Fixes #4959

Details and comments

The test cases were comparing a string which includes "pi" and I modify it to "π"

@alejomonbar alejomonbar requested review from a team, maddy-tod and nonhermitian as code owners October 8, 2020 19:26
@CLAassistant
Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ alejomonbar
❌ Jhon Montanez


Jhon Montanez seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@1ucian0
Copy link
Copy Markdown
Member

1ucian0 commented Oct 9, 2020

Jhon Montanez needs to sign CLA too

@alejomonbar
Copy link
Copy Markdown
Contributor Author

Jhon Montanez needs to sign CLA too

I'm Jhon Montanez. I didn't update my git hub email account in the terminal and I pull requested and I don't know how to solve it now :(!

@1ucian0
Copy link
Copy Markdown
Member

1ucian0 commented Oct 10, 2020

Maybe you want to remove it as co-author from the history?

@alejomonbar
Copy link
Copy Markdown
Contributor Author

Maybe you want to remove it as co-author from the history?

Yes, I'm trying to do that. But, I don't find the correct way to do that. Any advice?

@1ucian0
Copy link
Copy Markdown
Member

1ucian0 commented Oct 11, 2020

I think if you squash you will be able to remove the co-author.

@1ucian0
Copy link
Copy Markdown
Member

1ucian0 commented Oct 11, 2020

The problem the current solution is that π might be out of the charset for self.enconding an pi should be used in that case. Have a look to #4959 (comment)

@alejomonbar
Copy link
Copy Markdown
Contributor Author

The problem the current solution is that π might be out of the charset for self.enconding an pi should be used in that case. Have a look to #4959 (comment)

I tried to encode and decode with both encodings utf8 and cp437 and both are able to manage π. Do you want me to try another encode?

@1ucian0
Copy link
Copy Markdown
Member

1ucian0 commented Oct 12, 2020

Probably you are right and most of the charsets that have also have π. I was thinking that maybe obscure code pages like 866 might be a problem.. but probably they are problematic already...

@1ucian0
Copy link
Copy Markdown
Member

1ucian0 commented Oct 13, 2020

?

@alejomonbar
Copy link
Copy Markdown
Contributor Author

?

Sorry, @1ucian0, I closed this pull request and I create a new one. I couldn't figure out how to eliminate those commits with Jhon Montanez. The new pull request is this #5218

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.

Show pi in the text drawer as π

3 participants