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: Final touches patches for pre-release #1346

Merged
merged 1 commit into from
Jan 5, 2024

Conversation

daquinteroflex
Copy link
Collaborator

@daquinteroflex daquinteroflex commented Jan 3, 2024

  • pre-commit instructions re Weiliangs questions, fix that CLI command
  • further documentation build instructions as a result from my conversations with Tom.
  • python 3.8 support
  • (Maybe on this PR) lazy imports Fundamentally needs PR on pre/2.5 and post propagated to 2.6
  • Docstring updates
  • Coverage support

@daquinteroflex daquinteroflex force-pushed the dario/patches_pre_26_release branch 4 times, most recently from d45d7df to 9be1004 Compare January 5, 2024 10:57
@daquinteroflex daquinteroflex marked this pull request as ready for review January 5, 2024 10:58
Copy link
Collaborator

@tylerflex tylerflex left a comment

Choose a reason for hiding this comment

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

Looks good, @daquinteroflex , thank you!

I had a few minor comments / slight improvements to go over, but overall looks good and I'm approving so we can merge once they are all addressed.

.github/workflows/run_tests.yml Outdated Show resolved Hide resolved
docs/development/documentation.rst Show resolved Hide resolved
docs/development/usage.rst Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
]
jax = [
{version = ">=0.4.1,<=0.4.14", extras = ["cpu"], platform = "linux", source="jaxsource", optional = true},
{version = ">=0.4.1,<=0.4.14", extras = ["cpu"], platform = "darwin", source="jaxsource", optional = true},
{version = ">=0.4.13,<=0.4.14", extras = ["cpu"], platform = "win32", source="jaxsource", optional = true}
{version = ">=0.4.13,<=0.4.14", extras = ["cpu"], platform = "win32", python = "^3.9", source="jaxsource", 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.

we're already excluding any windows < 3.9 now? so maybe we dont need this python condition anymore?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So I think we're excluding it out of tests, I thought we were still supporting 3.8 where this would have to be included in the packaging?

Copy link
Collaborator

Choose a reason for hiding this comment

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

hm, I thought we were excluding windows with python 3.8 because jax didn't work at all? or was it just that we couldn't install it in our automatic test? feel free to keep it how you had it, maybe I just misunderstood

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it was because it was badly packaged for win py3.8. So I think the condition above just excludes that if I've understood the poetry requirements well.

tidy3d/web/api/webapi.py Outdated Show resolved Hide resolved
tidy3d/web/cli/develop.py Outdated Show resolved Hide resolved
@daquinteroflex daquinteroflex force-pushed the dario/patches_pre_26_release branch 4 times, most recently from 1b86d4f to 81aa5b7 Compare January 5, 2024 17:39
@daquinteroflex
Copy link
Collaborator Author

I think all tests are passing so let me know if you want a final look as I had to sort out some things with the fixes or if just going ahead!

@momchil-flex momchil-flex merged commit c456401 into pre/2.6 Jan 5, 2024
16 checks passed
@momchil-flex momchil-flex deleted the dario/patches_pre_26_release branch January 5, 2024 19:02
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