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

Adopt Poetry for development and build #47

Merged
merged 9 commits into from
Jul 17, 2021
Merged

Adopt Poetry for development and build #47

merged 9 commits into from
Jul 17, 2021

Conversation

paulmelnikow
Copy link
Member

No description provided.

@paulmelnikow paulmelnikow marked this pull request as ready for review July 14, 2021 18:43
@jbeard4
Copy link

jbeard4 commented Jul 16, 2021

tri-again's dependencies appear to install successfully from poetry on my local, but when I get a failure when I run the tests:

(base) jacob@pop-os:~/workspace/curvewise/tri-again$ ./dev.py test
==================================================================================== test session starts =====================================================================================
platform linux -- Python 3.9.1, pytest-6.2.4, py-1.10.0, pluggy-0.13.1
rootdir: /home/jacob/workspace/curvewise/tri-again
plugins: cov-2.12.1
collected 3 items

tri_again/test_collada.py F.                                                                                                                                                           [ 66%]
tri_again/test_color.py .                                                                                                                                                              [100%]

========================================================================================== FAILURES ==========================================================================================
________________________________________________________________________________________ test_example ________________________________________________________________________________________

    def test_example():
        scene = (
            Scene(point_radius=0.1)
            .add_meshes(
                shapes.cube(np.zeros(3), 3.0), shapes.cube(np.array([5.0, 0.0, 0.0]), 1.0)
            )
            .add_lines(
                Polyline(
                    np.array([[5.5, 0.5, 0.0], [5.5, 0.75, 0.0], [5.5, 0.5, -0.5]]),
                    is_closed=True,
                ),
                Polyline(
                    np.array([[1.5, 1.5, 0.0], [1.5, 1.75, 0.0], [1.5, 1.5, -0.5]]),
                    is_closed=True,
                ),
            )
            .add_points(np.zeros(3), np.array([5.0, 0.0, 0.0]))
            .add_points(
                np.array([0.0, 0.0, 0.3]), np.array([5.0, 0.0, 0.1]), color="SaddleBrown"
            )
            .add_points(
                np.array([0.0, 0.0, 0.6]), np.array([5.0, 0.0, 0.2]), color="SeaGreen"
            )
        )
>       scene.write("example.dae")

tri_again/test_collada.py:32:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
tri_again/_scene.py:90: in write
    dae.write(filename)
.venv/lib/python3.9/site-packages/collada/__init__.py:554: in write
    self.save()
.venv/lib/python3.9/site-packages/collada/__init__.py:524: in save
    o.save()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <Geometry id=mesh_geometry_0, 1 primitives>

    def save(self):
        """Saves the geometry back to :attr:`xmlnode`"""
        meshnode = self.xmlnode.find(tag('mesh'))
        for src in self.sourceById.values():
            if isinstance(src, source.Source):
                src.save()
>               if src.xmlnode not in meshnode.getchildren():
E               AttributeError: 'xml.etree.ElementTree.Element' object has no attribute 'getchildren'

.venv/lib/python3.9/site-packages/collada/geometry.py:232: AttributeError
================================================================================== short test summary info ===================================================================================
FAILED tri_again/test_collada.py::test_example - AttributeError: 'xml.etree.ElementTree.Element' object has no attribute 'getchildren'
================================================================================ 1 failed, 2 passed in 0.20s =================================================================================
Traceback (most recent call last):
  File "/home/jacob/workspace/curvewise/tri-again/./dev.py", line 88, in <module>
    cli()
  File "/home/jacob/workspace/curvewise/tri-again/.venv/lib/python3.9/site-packages/click/core.py", line 1137, in __call__
    return self.main(*args, **kwargs)
  File "/home/jacob/workspace/curvewise/tri-again/.venv/lib/python3.9/site-packages/click/core.py", line 1062, in main
    rv = self.invoke(ctx)
  File "/home/jacob/workspace/curvewise/tri-again/.venv/lib/python3.9/site-packages/click/core.py", line 1668, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/home/jacob/workspace/curvewise/tri-again/.venv/lib/python3.9/site-packages/click/core.py", line 1404, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/home/jacob/workspace/curvewise/tri-again/.venv/lib/python3.9/site-packages/click/core.py", line 763, in invoke
    return __callback(*args, **kwargs)
  File "/home/jacob/workspace/curvewise/tri-again/./dev.py", line 33, in test
    execute("pytest")
  File "/home/jacob/workspace/curvewise/tri-again/.venv/lib/python3.9/site-packages/executor/__init__.py", line 174, in execute
    return execute_prepared(ExternalCommand(*command, **options))
  File "/home/jacob/workspace/curvewise/tri-again/.venv/lib/python3.9/site-packages/executor/__init__.py", line 203, in execute_prepared
    command.start()
  File "/home/jacob/workspace/curvewise/tri-again/.venv/lib/python3.9/site-packages/executor/__init__.py", line 1441, in start
    self.start_once(**kw)
  File "/home/jacob/workspace/curvewise/tri-again/.venv/lib/python3.9/site-packages/executor/__init__.py", line 1508, in start_once
    self.wait(check=check)
  File "/home/jacob/workspace/curvewise/tri-again/.venv/lib/python3.9/site-packages/executor/__init__.py", line 1551, in wait
    self.check_errors(check=check)
  File "/home/jacob/workspace/curvewise/tri-again/.venv/lib/python3.9/site-packages/executor/__init__.py", line 1673, in check_errors
    raise self.error_type(self)
executor.ExternalCommandFailed: External command failed with exit code 1!

Command:
bash -c pytest

I found another issue that appears to be related to pip and python version dependency resolution: s3tools/s3cmd#1182

I looked into this, and my .venv was for python 3.9. It turns out bootstrap.zsh was using my conda base python 3.9 environment.

I then tried to conda activate pristine to try a python 3.7 environment, and then ran bootstrap.zsh. Again, all dependencies appeared to install fine. But when I attempted to run dev.py, I got the following error:

(pristine) jacob@pop-os:~/workspace/curvewise/tri-again$ ./dev.py test
Traceback (most recent call last):
  File "./dev.py", line 4, in <module>
    import click
ModuleNotFoundError: No module named 'click'

So then I attempted explicitly activate my .venv with source .venv/bin/activate, and then ran python dev.py test. This time the tests passed with 3 successes and 21 warnings:

(.venv) (pristine) jacob@pop-os:~/workspace/curvewise/tri-again$ python dev.py test
==================================================================================== test session starts =====================================================================================
platform linux -- Python 3.7.9, pytest-6.2.4, py-1.10.0, pluggy-0.13.1
rootdir: /home/jacob/workspace/curvewise/tri-again
plugins: cov-2.12.1
collected 3 items

tri_again/test_collada.py ..                                                                                                                                                           [ 66%]
tri_again/test_color.py .                                                                                                                                                              [100%]

====================================================================================== warnings summary ======================================================================================
tri_again/test_collada.py::test_example
tri_again/test_collada.py::test_example
tri_again/test_collada.py::test_example
tri_again/test_collada.py::test_example
tri_again/test_collada.py::test_example
tri_again/test_collada.py::test_example
tri_again/test_collada.py::test_example
  /home/jacob/workspace/curvewise/tri-again/.venv/lib/python3.7/site-packages/collada/geometry.py:232: DeprecationWarning: This method will be removed in future versions.  Use 'list(elem)' or iteration over elem instead.
    if src.xmlnode not in meshnode.getchildren():

tri_again/test_collada.py::test_example
tri_again/test_collada.py::test_example
tri_again/test_collada.py::test_example
tri_again/test_collada.py::test_example
tri_again/test_collada.py::test_example
tri_again/test_collada.py::test_example
tri_again/test_collada.py::test_example
  /home/jacob/workspace/curvewise/tri-again/.venv/lib/python3.7/site-packages/collada/geometry.py:294: DeprecationWarning: This method will be removed in future versions.  Use 'list(elem)' or iteration over elem instead.
    if prim.xmlnode not in meshnode.getchildren():

tri_again/test_collada.py::test_example
tri_again/test_collada.py::test_example
tri_again/test_collada.py::test_example
tri_again/test_collada.py::test_example
tri_again/test_collada.py::test_example
tri_again/test_collada.py::test_example
tri_again/test_collada.py::test_example
  /home/jacob/workspace/curvewise/tri-again/.venv/lib/python3.7/site-packages/collada/geometry.py:299: DeprecationWarning: This method will be removed in future versions.  Use 'list(elem)' or iteration over elem instead.
    for child in meshnode.getchildren():

-- Docs: https://docs.pytest.org/en/stable/warnings.html
=============================================================================== 3 passed, 21 warnings in 0.16s ===============================================================================

So, I think there are a couple of issues here:

  1. First, with python 3.9, it seems to be installing a newer version of collada or lxml, where getchildren method is removed.
  2. Next, do we want the user to explicitly control which python base environment/python version they are using? I thought we were using a different approach in e.g. armscyence where we normalized this for them to use the pristine 3.7 environment.
  3. Finally, while in the pristine environment, it seems that the shebang is not working on the dev.py script, as it was not able to import the click dependency from the venv. Instead, I had to manually activate the 3.7 venv and run python dev.py in order for it to import its dependencies.

Let me know if you have any questions about this. Thanks.

@paulmelnikow
Copy link
Member Author

paulmelnikow commented Jul 16, 2021

First, with python 3.9, it seems to be installing a newer version of collada or lxml, where getchildren method is removed.

Right, this is documented in #25 and also in the pyproject.toml which should generate a requires_python>=3.7,<3.9. We're waiting for a pycollada release with this fix: pycollada/pycollada#97 (comment)

Next, do we want the user to explicitly control which python base environment/python version they are using? I thought we were using a different approach in e.g. armscyence where we normalized this for them to use the pristine 3.7 environment.

This is an OSS repository so it doesn't depend on anything Curvewise-specific. It's the developer's responsibility to install Python <3.9 before running bootstrap.zsh. I'll add that to the readme. (We could also include a version check in bootstrap.zsh though I'm not sure it's worth the effort.)

Finally, while in the pristine environment, it seems that the shebang is not working on the dev.py script, as it was not able to import the click dependency from the venv. Instead, I had to manually activate the 3.7 venv and run python dev.py in order for it to import its dependencies.

Yup, the first install has to use poetry install, and starting with the second one you can use ./dev.py install. This is how bootstrap.zsh works, too, in the repos like this one which require extras for development.

Added:

I then tried to conda activate pristine to try a python 3.7 environment, and then ran bootstrap.zsh. Again, all dependencies appeared to install fine. But when I attempted to run dev.py, I got the following error:

This might be a quirk (or bug) in Conda, and possibly a difference between 1.1.7 and 1.2.0. Let's look at it together today.

@jbeard4
Copy link

jbeard4 commented Jul 16, 2021

Thanks for your comments. I think the only blocker to merge here is that the shebang in ./dev.py did not work as expected. We probably want to normalize this behavior as a part of normalizing the local python environment, so that the way we invoke dev.py works consistently across projects. Let's look into this today.

@paulmelnikow
Copy link
Member Author

Thanks for your comments. I think the only blocker to merge here is that the shebang in ./dev.py did not work as expected. We probably want to normalize this behavior as a part of normalizing the local python environment, so that the way we invoke dev.py works consistently across projects. Let's look into this today.

Okay. The expected behavior is that, as dev.py depends on click, executor, and sometimes others, it will not work until poetry install is first run. This behavior is consistent across all the projects.

Copy link

@jbeard4 jbeard4 left a comment

Choose a reason for hiding this comment

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

Discussed solution on slack.

@paulmelnikow paulmelnikow merged commit 5387519 into main Jul 17, 2021
@paulmelnikow paulmelnikow deleted the poetry branch July 17, 2021 22:10
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