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

Create metapackage #35

Merged
merged 34 commits into from
Aug 4, 2022
Merged

Create metapackage #35

merged 34 commits into from
Aug 4, 2022

Conversation

RobPasMue
Copy link
Member

Resolving #34 #13

@RobPasMue RobPasMue self-assigned this Jul 29, 2022
This was linked to issues Jul 29, 2022
@RobPasMue
Copy link
Member Author

Project adapted to pyproject.toml.
Working on setting up CI/CD now.

@RobPasMue
Copy link
Member Author

RobPasMue commented Aug 1, 2022

Current situation:

  • I have cleaned up the repository in order to use a single pyproject.toml file with poetry (this allows us to remove the requirement files and lets dependabot update the reqs regularly)
  • We have created a "core" install and "extras" installs (for fluent-all, mapdl-all, all etc.)
  • Vale has been implemented as well for the docs (even though it's only the main README.rst and a simple index.rst)
  • Pre-commit implemented for style checks
  • A new versioning for this package has been conceived (YYYY.X.dev0 for main where YYYY is the release year and X is the release within the year)
  • A proper CI workflow has been implemented in which we are testing: code style, docs style, core build, "extras" build (nothing depends on it if it fails), packaging stage and release.

Future works:

  • It looks like Python 3.10 is failing due to VTK in some of our PyAnsys libraries. I will need to look into that before merging so that everything works. Probably will have to open PRs in those libraries where Python 3.10 is not supported.
  • We should also implement the automatic "milestone" generation. I already have some "local" proposal for this but I will have to handle it in a different PR (this one is already growing too much)

This being said, I'll leave this PR for review.

@RobPasMue RobPasMue requested a review from MaxJPRey August 1, 2022 12:19
@RobPasMue RobPasMue marked this pull request as ready for review August 1, 2022 12:19
@RobPasMue RobPasMue added this to the v2023R1 milestone Aug 1, 2022
Copy link
Member

@jorgepiloto jorgepiloto left a comment

Choose a reason for hiding this comment

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

This looks really good, @RobPasMue. Just a couple of things:

  • No need to have a _version.py file. Let us have everything inside __init__.py. This allows also to simplify the very first lines in conf.py.

  • I agree on using poetry for this. It would be nice to commit the lock.file too.

  • Additionally to the wheel and source artifacts, we could ship the "wheelhouse", as PyMAPDL does, see these lines.

.github/workflows/ci-build.yml Outdated Show resolved Hide resolved
@RobPasMue
Copy link
Member Author

This looks really good, @RobPasMue. Just a couple of things:

  • No need to have a _version.py file. Let us have everything inside __init__.py. This allows also to simplify the very first lines in conf.py.
  • I agree on using poetry for this. It would be nice to commit the lock.file too.
  • Additionally to the wheel and source artifacts, we could ship the "wheelhouse", as PyMAPDL does, see these lines.

Thank you for your comments @jorgepiloto I fully agree with all of them. Let me change them :)

@RobPasMue
Copy link
Member Author

RobPasMue commented Aug 1, 2022

Problems encountered with ansys-mapdl-reader and macOS for all Python versions. Pinging @akaszynski and @germa89 for discussion. It looks like (from the CI logs) that there are only ansys-mapdl-reader packages for macOS available up to 0.51.7... Kind of weird. I will look into it tomorrow.

Error message:

ERROR: Could not find a version that satisfies the requirement ansys-mapdl-reader==0.51.14; extra == "mapdl-all" or extra == "all" (from pyansys[mapdl-all]) (from versions: 0.50.0, 0.50.1, 0.50.2, 0.50.3, 0.50.4, 0.50.5, 0.50.6, 0.50.7, 0.50.8, 0.50.9, 0.50.10, 0.50.11, 0.50.12, 0.50.13, 0.50.14, 0.50.15, 0.50.16, 0.51.0, 0.51.1, 0.51.2, 0.51.3, 0.51.4, 0.51.5, 0.51.6, 0.51.7)
ERROR: No matching distribution found for ansys-mapdl-reader==0.51.14; extra == "mapdl-all" or extra == "all"

pyproject.toml Outdated Show resolved Hide resolved
Co-authored-by: Maxime Rey <[email protected]>
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
Comment on lines +42 to +43
ansys-dpf-core = "==0.4.2"
ansys-dpf-post = "==0.2.2"
Copy link
Contributor

Choose a reason for hiding this comment

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

These will have to be updated. As it stands, pydpf-core==0.5.0 is for 2022R2, which we're still waiting on.

Copy link
Member Author

@RobPasMue RobPasMue Aug 4, 2022

Choose a reason for hiding this comment

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

At the moment, the latest available one is 0.4.2 (I refer to the released versions, not pre-released). As soon as they publish 0.5.0, dependabot will let us know


# Optional dependencies
ansys-mapdl-reader = {version = "==0.51.14", optional = true}
# ansys-fluent-visualization = {version = "==0.4.0", optional = true}
Copy link
Contributor

Choose a reason for hiding this comment

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

We'll have to bring this up in a discussion tomorrow.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed. Planning on. There is an issue open as well. ansys/pyfluent-visualization#94

@RobPasMue
Copy link
Member Author

@jorgepiloto @MaxJPRey @akaszynski I think, as it is, we can merge the PR. Can you please review it (and approve) when possible? Remaining open points already have issues created in this same repo

@jorgepiloto
Copy link
Member

Sure, let me do a quick review on this, @RobPasMue 👍🏽

doc/source/conf.py Outdated Show resolved Hide resolved
Copy link
Member

@jorgepiloto jorgepiloto left a comment

Choose a reason for hiding this comment

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

Just one change request regarding the conf.py file and a tiny comment. This looks really good. This solution is really smart, congratulations @RobPasMue 👏🏽

@MaxJPRey
Copy link
Contributor

MaxJPRey commented Aug 4, 2022

Well done @RobPasMue. This is excellent.

@RobPasMue RobPasMue merged commit 54a8de5 into main Aug 4, 2022
@RobPasMue RobPasMue deleted the feat/create-metapackage branch August 4, 2022 09:57
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.

Create metapackage Consider using pyproject.toml
4 participants