-
Notifications
You must be signed in to change notification settings - Fork 92
refactor: adopt _src pattern and migrate core components
#763
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
base: 0.3
Are you sure you want to change the base?
Conversation
- Restructured the repository to separate internal logic (`pina/_src`) from the public API. - Migrated central components (Graph, LabelTensor, Operator, Trainer) into `pina/_src/core/` for better organization. - Updated all subpackage `__init__.py` files to expose a clean, flat import surface for users. - Standardized module docstrings across Solvers, Models, Equations, Domains, and Callbacks to improve documentation quality. - Optimized internal dictionary handling in tensor storage functions.
|
Hi @adendek ! Thanks for the PR. At the moment, all tests are failing, probably due to an import error. Can you check it? Also, the |
|
I'fixed the UT and formatting. I guess they should be fine now. I'll need a little bit more time to update the docs. |
This branch was created to be able to run all tests using on my fork.
|
Hello @dario-coscia, I have successfully refactored the Sphinx documentation to align with the new As shown in the latest CI run executed on my fork, the
So the PR is ready to be reviewed. |
|
Dear @adendek, thanks a lot! But all the other unittest are green. I think there is only something wrong in the action environment. |
|
Hello @ndem0, This is one of the most bizarre CI discrepancies I have encountered. I have reviewed the workflow configuration, and while the logic seems sound, I noticed a few points worth investigating. Workflow Improvement (Explicit Versioning). Currently, the coverage job does not explicitly define a Python version, meaning it defaults to whatever is pre-installed on the runner (often the latest available). We should improve this by explicitly pinning the version to ensure consistency with the unittests job: coverage:
runs-on: ubuntu-latest
strategy:
matrix:
python-version: ['3.10'] # Explicitly defined
steps:
- uses: actions/checkout@v4
- uses: actions/setup-python@v5
with:
python-version: ${{ matrix.python-version }}
# ... remaining stepsEnvironmental Comparison Even without the pin, the software environment in the failing run looks reasonable and matches the standard: The Fork Success What is most confusing is that the exact same code passed all checks on my personal fork. The logs show identical versions of Python, pytest, and the coverage plugin. Before I start refactoring the workflow structure to hunt for a phantom bug, could you please manually re-trigger the coverage test? I want to rule out a transient "fluke" or a runner-specific environment glitch.
If it fails again, I will dive deeper into the workflow configuration. |
|
Hi @adendek! Green flag from myside, great job! It will be very useful for the future of the package. |
GiovanniCanali
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might be a naive question, but why do we have so many empty __init__.py files in the src directory? Are they still needed?
I guess it will be easier if you do this during the merge process. You can select the options to squash merge.
I am not even able to make the merge on my own.
The It signals to the Python interpreter that the directory contains modules that can be imported. Without it (in Python 3.3+), the folder is treated as a Namespace Package, which can lead to unpredictable behavior when multiple directories share the same name across the sys.path. Essential tools in the PINA pipeline—specifically Sphinx for documentation and |




Description
This PR resolves #756 by implementing a major structural refactor. Following the design patterns of modern scientific libraries like JAX and Optax, it introduces a
pina/_srcdirectory to house internal logic while exposing a clean, flat public API.API Stability Notice
The public interface remains unchanged. Existing user scripts and external calls to PINA classes will continue to work as expected. This refactor purely reorganizes the internal source code and "hoists" definitions to their respective package levels for cleaner access.
Key Changes
pina._src.core/module.Checklist