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

📦 BUILD: Python 3.12 compatibility #1622

Merged
merged 1 commit into from
Apr 19, 2024
Merged

Conversation

daquinteroflex
Copy link
Collaborator

Related to:

#1620
#1317
#1593

@daquinteroflex daquinteroflex force-pushed the dario/python_3_12 branch 3 times, most recently from 5d6bdfa to d33814b Compare April 17, 2024 14:26
@momchil-flex
Copy link
Collaborator

So the only remaining issue is mac with python 3.12 and gdstk? Probably @lucas-flexcompute needs to build a new wheel or something?

@lucas-flexcompute
Copy link
Collaborator

I did not notice macOS was missing python 3.11 and 3.12! I think it's just a matter of updating the GH actions for CIBW. I'm checking now: https://github.com/heitzmann/gdstk/actions/runs/8730179893

@lucas-flexcompute
Copy link
Collaborator

The 3.12 versions are now on PyPI.

@momchil-flex
Copy link
Collaborator

Seems like now xarray errors...

@daquinteroflex daquinteroflex changed the title Further python 3.12 updates 📦 BUILD: Python 3.12 compatibility Apr 18, 2024
@daquinteroflex
Copy link
Collaborator Author

Thanks both for your help and advice! Think it's passing now and seems reasonable imo. However, maybe we want to merge #1593 before this one as it depends on those updates too.

Copy link
Collaborator

@momchil-flex momchil-flex left a comment

Choose a reason for hiding this comment

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

Not sure whether to put this in rc1 or not. I guess it may be nice to test right away...

pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Show resolved Hide resolved
sax = {version="*", optional = true}
vtk = {version="<=9.2.6", optional = true}
vtk = {version=">=9.2.6", optional = true}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Similarly, are you sure about this? @dbochkov any thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

Should be ok starting with 2.7

# subprocess.call(["python", "-c", "pass"])
# python_load_time += time.time() - s
#
# print(f"average tidy3d load time = {(tidy3d_load_time - python_load_time) / n}")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Whoops that was temporary to speed up testing, but now have brought it back.

@momchil-flex
Copy link
Collaborator

AHHH the dangers of force pushing!!! Yesterday I had done git push -f origin dario/python_3_12 to remove the commit that excluded the Mac OS text. Today I just completely not on purpose ran that command again (I was looking to repeat a different command) and it overwrote your branch!

I assume you have a copy locally so hopefully no big deal... Sorry!

@daquinteroflex
Copy link
Collaborator Author

No worries haha! All feedback implemented.

I'm ok if we want to add it to rc1. I believe the main doubt were the jax upgrade questions @tylerflex fyi

@momchil-flex
Copy link
Collaborator

Well, better to get feedback right away than when we do the official release :)

@momchil-flex momchil-flex merged commit 1e7568e into pre/2.7 Apr 19, 2024
16 checks passed
@momchil-flex momchil-flex deleted the dario/python_3_12 branch April 19, 2024 19:56
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.

6 participants