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

Compatibility with submodules and docstrings #18

Open
wants to merge 28 commits into
base: develop
Choose a base branch
from

Conversation

alfonsoSR
Copy link

Goal

To make the current develop branch of the feedstock compatible with tudat-team/tudatpy#159.

Changelog

  • Removed sysconfig patch from recipe
  • Updated import paths and tests in meta.yaml

Tutdat & TudatPy versions

The setup in this PR has been tested for

  • Tudat: 2.13.0.dev16
  • Tudatpy: 228601aa1f6efba0f2443f0621cd8bf8dc939a31 (Commit SHA)

Copy link
Member

@geoffreygarrett geoffreygarrett left a comment

Choose a reason for hiding this comment

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

Perhaps let the tests run before merging?

@@ -10,8 +10,6 @@ package:
source:
git_url: https://github.com/tudat-team/tudatpy.git
git_rev: {{ git_rev }}
patches:
- use_sysconfig_for_python_paths.patch
Copy link
Member

Choose a reason for hiding this comment

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

Is Python 3.12 passing without this now, or has this been added to the tudatpy source?

Copy link
Author

Choose a reason for hiding this comment

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

I increased the minimum required CMake version for tudatpy to be able to use the "new" method for finding Python, so we don't need that part of the YACMASetup script anymore

Copy link
Author

Choose a reason for hiding this comment

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

When you say 3.12 I assume you refer to the one that is used to generate the macOS distributions, right? Because we only support 3.9 to 3.11 right now (until I figure out why pydantic does not support 3.12 or why are we forcing the use of a version that doesn't)

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I'm not up to date on what you're working towards with CMake at the moment, but I added that patch so that Python 3.12 was supported by tudatpy and found using YACMA.

Feel free to provide links and a summary of what you're fixing.

Comment on lines -121 to -122
- python -c "from tudatpy.io import get_resource_path; print(get_resource_path())"
- python -c "from tudatpy.io import save2txt"
Copy link
Member

Choose a reason for hiding this comment

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

I know someone is going to be happy about this change. @DominicDirkx

@alfonsoSR
Copy link
Author

alfonsoSR commented Oct 2, 2024

Perhaps let the tests run before merging?

What do you mean? @geoffreygarrett

@geoffreygarrett
Copy link
Member

Perhaps let the tests run before merging?

What do you mean? @geoffreygarrett

You pinged me with a review request, unless that was a mistake. I was suggesting you let the tests run on Azure so I can do that.

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.

2 participants