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

Simplify setup.py; enable build for Windows, MacOS and every major Linux system; add actions to build wheel files #127

Merged
merged 1 commit into from
Oct 3, 2022

Conversation

semiversus
Copy link
Contributor

The major goal for this pull request is to be able to compile pyoptools for Windows, MacOS and any major Linux system.

setup.py

This file was simplified and uses the Cython functionality to compile the given files. There is a problem with MacOS and Windows compilers not supporting inline on data declarations - in this case the CYTHON_INLINE macro is redefined.

Github Actions

With the added workflow packages are automatically build for Windows, MacOS and Linux for Python3.7, 3.8, 3.9 and 3.10.

Others

  • mat_eq.py needs an 'utf-8' encoding argument when opening files. Otherwise Windows falls back to CP-1252 encoding.
  • field.pyx changes definitions for I and cexp which is not working with Windows compiler

Next steps (will be pull requests, when this one is accepted)

  • Automatically push packages to pypi on releases (Imagine pip install pyoptools on any platform... :-) )
  • Redo installation documentation (as it will be much easier at that state)
  • Fix unittests and run them in github actions
  • Cleanup of old (and probably unused) scripts and files in root folder
  • Make fixes to use Cython language_level "3" (currently it's based on Python 2 syntax)
  • ...

@rdgraham
Copy link
Contributor

rdgraham commented Oct 2, 2022

This is great. I checked out the pull request and installed via pip in a new conda environment.

Installed without issue. First time I have had operational pyoptools in windows.

As of now I still don't have the 3d view via pythreejs in jupyter working, even though I have installed and enabled it. Getting 404 errors in the terminal when I try to use plot3D. Though I suspect that is probably a conda problem or something not directly related to pyoptools.

Thanks!

@semiversus
Copy link
Contributor Author

I did the following in my conda environment and it was working with that:

pip install jupyterlab pythreejs
jupyter nbextension install --py pythreejs
jupyter nbextension enable pythreejs --py
jupyter-lab

@ramezquitao ramezquitao merged commit b6e338b into cihologramas:master Oct 3, 2022
@rdgraham
Copy link
Contributor

rdgraham commented Oct 4, 2022

Thank you, with those exact commands I have it working. Seems like it should be equivalent to what I did, but anyway ..

Now that I have the display working I am seeing a strange issue with this build.

Looks like what is happening is that rotations are not being applied to surfaces correctly. For example the cylindrical 'wall' sections of lenses are not being rotated correctly as shown in the screenshot below from the tutorial.

I have tried the latest build on linux and am not seeing this issue. Is this maybe a strange numerical showing up with the different cython compiler or something? Anyone else seeing this?

I can split this off into a new issue if there isn't some immediate fix like a compiler flag or something.

image

@semiversus
Copy link
Contributor Author

Could you please try one of the attached wheel files (depenting on your python version). You can use pip install pyoptools-0.1.1-cp37-cp37m-win_amd64.whl (example for Python 3.7). These are the wheel files created by the github actions workflow.

pyoptools-0.1.1_windows_wheels.zip

@rdgraham
Copy link
Contributor

rdgraham commented Oct 5, 2022

Thanks for that, I downloaded and tried it out with python 3.10, installed with the --force-reinstall option and that went without issue.

But I am still seeing the same problem.

On looking at the polychromatic prism example, it seems like the actual underlying calculation and raytracing is probably correct, and the problem is just in the rendering of the surfaces. So this might be a openGL problem or problem in ipynbplotutils.py renderer. I will try some different versions and see if I can debug this when I get a chance.

image

@semiversus
Copy link
Contributor Author

Ok, thanks for testing. I'll have a look too. I also guess it's a rendering problem. We may put this into a issue by it's own.

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