molstar / mol-view-spec

https://molstar.org/mol-view-spec/
MIT License
8 stars 1 forks source link

Fixed union with 'Union' type from 'typing' to support python<3.10 #16

Closed chdominguez closed 6 months ago

chdominguez commented 7 months ago

I have a project built with python 3.9, and initially molviewspec was not working. I found that the nodes.py module was using | notation to type hint unions, and this does not work on python versions prior to 3.10. Thankfully, the typing module includes a Union class to represent such types, and this works also on older versions of python.

Without this change, molviewspec simply does not work on older versions of python than 3.10

JonStargaryen commented 7 months ago

Hi Christian,

thanks for your interest and contribution. Other parts of the library make use of | as well (specifically types of method parameters in builder.py). Do these cause issues too?

chdominguez commented 7 months ago

On my brief testing with the library that's the only issue I found. But still I think we should change all | for Union just to improve compatibility. I will take a look and update the PR!

chdominguez commented 6 months ago

The issue is not present on builder.py because annotations are imported from __future__ which allows for Unions on python 3.9.

Other important change that I made is to froze the pydantic version to <2 (which was automatically installed when creating a conda environment with the environment.yaml file due to other requirements). This is necessary as pydantic>2introduced some changes on the validator root class which is not backwards compatible with pydantic<2.

JonStargaryen commented 6 months ago

Great, many thanks!