Skip to content

Detect py arch - #961

Merged
filmor merged 5 commits into
pythonnet:masterfrom
jmlidbetter:detect-py-arch
Oct 1, 2019
Merged

filmor merged 5 commits into
pythonnet:masterfrom
jmlidbetter:detect-py-arch

Conversation

@jmlidbetter

Copy link
Copy Markdown
Contributor

What does this implement/fix? Explain your changes.

As described in Issue #950, the size of python's longs can vary depending on platform. This changes allows pythonnet to determine at runtime the architecture of the python interpreter and select the correct size of long/int for the affected methods.

Does this close any currently open issues?

#950

Any other comments?

No

Checklist

Check all those that are applicable and complete.

  • Make sure to include one or more tests for your change
  • If an enhancement PR, please create docs and at best an example
  • Add yourself to AUTHORS
  • Updated the CHANGELOG

Comment thread src/runtime/runtime.cs Outdated
Comment thread src/runtime/runtime.cs
else
// Couldn't determine interpreter arch, try 64 bit
return PyLong_FromUnsignedLong64(Convert.ToUInt64(value));
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is kind of fine, just use the already provided Runtime.Is32Bit and Runtime.IsWindows instead.

@filmor

filmor commented Sep 26, 2019

Copy link
Copy Markdown
Member

Thanks for the contribution, the fix to the forwarded functions should do the trick, just use the already existing properties instead of trying to draw this from Python :)

Comment thread src/runtime/runtime.cs Outdated
@filmor
filmor merged commit 60e6045 into pythonnet:master Oct 1, 2019
AlexCatarino pushed a commit to QuantConnect/pythonnet that referenced this pull request Jun 27, 2020
* Gets size of C long from Is32Bit and IsWindows
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