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

ga: Move wheel/sdist creation to install-pnl action #3085

Merged
merged 1 commit into from
Nov 7, 2024

Conversation

jvesely
Copy link
Collaborator

@jvesely jvesely commented Oct 27, 2024

Use wheel package to install psyneulink instead of using editable install.
Provide paths in install-pnl action outputs.
Use install-pnl outputs to upload the hwheel/sdist.

@jvesely jvesely requested a review from kmantel October 27, 2024 04:25
Copy link

This PR causes the following changes to the html docs (ubuntu-latest-3.11):

Only in docs-base/_images: Composition_XOR_animation.gif

...

See CI logs for the full diff.

@jvesely
Copy link
Collaborator Author

jvesely commented Oct 27, 2024

This passes CI but still has issues.
Issue 1:
The root_dir cannot be set in _version.py
show graph doesn't work:

  File "/home/runner/work/_temp/_venv/lib/python3.11/site-packages/psyneulink/core/globals/context.py", line 742, in wrapper
    return func(*args, context=context, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/work/_temp/_venv/lib/python3.11/site-packages/psyneulink/core/compositions/composition.py", line 11670, in learn
    result = runner.run_learning(
             ^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/work/_temp/_venv/lib/python3.11/site-packages/psyneulink/library/compositions/compositionrunner.py", line 348, in run_learning
    self._composition.run(inputs=minibatched_input,
  File "/home/runner/work/_temp/_venv/lib/python3.11/site-packages/psyneulink/core/globals/context.py", line 742, in wrapper
    return func(*args, context=context, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/work/_temp/_venv/lib/python3.11/site-packages/psyneulink/core/compositions/composition.py", line 11209, in run
    self._set_up_animation(context)
  File "/home/runner/work/_temp/_venv/lib/python3.11/site-packages/psyneulink/core/compositions/composition.py", line 13668, in _set_up_animation
    self._show_graph._set_up_animation(context)
  File "/home/runner/work/_temp/_venv/lib/python3.11/site-packages/psyneulink/core/compositions/showgraph.py", line 2816, in _set_up_animation
    from psyneulink._version import root_dir
ImportError: cannot import name 'root_dir' from 'psyneulink._version' (/home/runner/work/_temp/_venv/lib/python3.11/site-packages/psyneulink/_version.py)

This is because _version gets updated when generating sdist or wheel:

...
copying tutorial/PsyNeuLink Tutorial.ipynb -> psyneulink-0.15.1.0+376.gf5f8f2d348/tutorial
copying psyneulink.egg-info/SOURCES.txt -> psyneulink-0.15.1.0+376.gf5f8f2d348/psyneulink.egg-info
Writing psyneulink-0.15.1.0+376.gf5f8f2d348/setup.cfg
UPDATING psyneulink-0.15.1.0+376.gf5f8f2d348/psyneulink/_version.py
set psyneulink-0.15.1.0+376.gf5f8f2d348/psyneulink/_version.py to '0.15.1.0+376.gf5f8f2d348'
creating dist
Creating tar archive
removing 'psyneulink-0.15.1.0+376.gf5f8f2d348' (and everything under it)

no longer an issue since #3094

2.
We cannot use root_dir to store any generated files.
The above is good, the root would resolve to .../lib/python3.11/site-packages/psyneulink/ or something similar. We definitely shouldn't save test-generated files there.

fixed in #3094

3.
The documentation jobs do not fail even if the animations do

fixed in #3087

@jvesely
Copy link
Collaborator Author

jvesely commented Oct 28, 2024

@kmantel
Copy link
Collaborator

kmantel commented Oct 31, 2024

Does 2 break this also, or is it just not great to have the files there? Anyway, it's not harder to just do both - root_dir is only used in the one place in showgraph.py, and it can be determined as needed, avoiding the site-packages dir if pnl is installed there.

Copy link

github-actions bot commented Nov 6, 2024

This PR causes the following changes to the html docs (ubuntu-latest-3.11):

No differences!

...

See CI logs for the full diff.

@jvesely jvesely marked this pull request as ready for review November 6, 2024 14:46
@jvesely jvesely added the CI Continuous Integration label Nov 6, 2024
Copy link

github-actions bot commented Nov 6, 2024

This PR causes the following changes to the html docs (ubuntu-latest-3.11):

No differences!

...

See CI logs for the full diff.

@jvesely
Copy link
Collaborator Author

jvesely commented Nov 6, 2024

Does 2 break this also, or is it just not great to have the files there? Anyway, it's not harder to just do both - root_dir is only used in the one place in showgraph.py, and it can be determined as needed, avoiding the site-packages dir if pnl is installed there.

The installation directory can be in the system root (e.g., /usr/lib64/Python3.11/site-packages), which we don't have permission to write to. Even if we do have permissions (e.g., ~/pnl-venv/lib64/Python3.11/site-packages), it's not a user-friendly location to put generated files.

Copy link

github-actions bot commented Nov 6, 2024

This PR causes the following changes to the html docs (ubuntu-latest-3.11):

No differences!

...

See CI logs for the full diff.

Use wheel package to install psyneulink instead of using editable
install.
Provide paths in install-pnl action outputs.
Use install-pnl outputs to upload the hwheel/sdist.

Signed-off-by: Jan Vesely <[email protected]>
Copy link

github-actions bot commented Nov 7, 2024

This PR causes the following changes to the html docs (ubuntu-latest-3.11):

diff -r docs-base/ModulatoryProjection.html docs-head/ModulatoryProjection.html
272c272
< <a class="reference internal" href="Port.html"><span class="doc">Port</span></a> are listed in the Port’s <a class="reference internal" href="ParameterPort.html#psyneulink.core.components.ports.parameterport.ParameterPort.mod_afferents" title="psyneulink.core.components.ports.parameterport.ParameterPort.mod_afferents"><code class="xref any py py-attr docutils literal notranslate"><span class="pre">mod_afferents</span></code></a> attribute.</p>
---
> <a class="reference internal" href="Port.html"><span class="doc">Port</span></a> are listed in the Port’s <a class="reference internal" href="Mechanism.html#id7" title="psyneulink.core.components.mechanisms.mechanism.Mechanism_Base.mod_afferents"><code class="xref any py py-attr docutils literal notranslate"><span class="pre">mod_afferents</span></code></a> attribute.</p>
diff -r docs-base/Port.html docs-head/Port.html
616c616
< <code class="xref any docutils literal notranslate"><span class="pre">PathWayProjections</span></code> (listed in its <a class="reference internal" href="#psyneulink.core.components.ports.port.Port_Base.path_afferents" title="psyneulink.core.components.ports.port.Port_Base.path_afferents"><code class="xref any py py-attr docutils literal notranslate"><span class="pre">path_afferents</span></code></a> attribute) as the variable for its
---
> <code class="xref any docutils literal notranslate"><span class="pre">PathWayProjections</span></code> (listed in its <a class="reference internal" href="Mechanism.html#id6" title="psyneulink.core.components.mechanisms.mechanism.Mechanism_Base.path_afferents"><code class="xref any py py-attr docutils literal notranslate"><span class="pre">path_afferents</span></code></a> attribute) as the variable for its
diff -r docs-base/TransferFunctions.html docs-head/TransferFunctions.html
2199c2199
< <p><a class="reference internal" href="OptimizationFunctions.html#psyneulink.core.components.functions.optimizationfunctions.GradientOptimization.bounds" title="psyneulink.core.components.functions.optimizationfunctions.GradientOptimization.bounds"><code class="xref any py py-attr docutils literal notranslate"><span class="pre">bounds</span></code></a> – specifies the lower and upper limits of the result;  if there are none, the attribute is set to
---
> <p><a class="reference internal" href="#psyneulink.core.components.functions.transferfunctions.Exponential.bounds" title="psyneulink.core.components.functions.transferfunctions.Exponential.bounds"><code class="xref any py py-attr docutils literal notranslate"><span class="pre">bounds</span></code></a> – specifies the lower and upper limits of the result;  if there are none, the attribute is set to

...

See CI logs for the full diff.

@jvesely jvesely merged commit 41b254d into PrincetonUniversity:devel Nov 7, 2024
36 checks passed
@jvesely jvesely deleted the ci-wheel branch November 7, 2024 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous Integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants