feat: Support numpy types #209
Conversation
kmsquire
force-pushed
the
feature/numpy
branch
3 times, most recently
from
55dc631
to
8f8e21a
Compare
|
Hi @kmsquire! This PR is awesome! Let me review it on this weekend 👍 |
|
Tests seem to be failing on 3.7. Locally they pass for me on 3.9 on macos, but maybe I'm doing something that isn't compatible with 3.7. I'll try to take a look. |
|
I think I fixed the issues on 3.7 (and 3.8). The fix might be a little hacky, so I'm very open to changes. |
|
It appears Github Actions uses CPython 3.7 instead of pypy>=3.7. Let me fix it first and see if the poetry error disappears.
https://github.com/yukinarit/pyserde/runs/5701449127?check_suite_focus=true |
|
Hi, @kmsquire I've fixed to use pypy 3.8. Could you rebase or merge the change and push again? It may resolve the CI issue. |
|
Looks like numpy 1.21.1 wheel is not provided for python 3.10? https://pypi.org/project/numpy/1.21.1/#files @kmsquire |
It looks like numpy 1.22.3 only supports python 3.8-3.10 (not 3.7) (link), so poetry probably picked a version that (it thought) supported >3.7. I'll try to see if a conditional install will work (1.21.x on 3.7, 1.22.x on >=3.8). |
Codecov Report
@@ Coverage Diff @@
## master #209 +/- ##
==========================================
- Coverage 92.56% 91.47% -1.09%
==========================================
Files 10 11 +1
Lines 1250 1338 +88
Branches 272 286 +14
==========================================
+ Hits 1157 1224 +67
- Misses 59 78 +19
- Partials 34 36 +2
Continue to review full report at Codecov.
|
|
Okay, I fixed the numpy install issue. I hadn't run
I think all that is remaining is to add some more tests to fill out test coverage (and whatever else you comment on). |
|
As a note to self when I have time, here is possible way to test the code run if |
Hi, to be clear: the formatting issue doesn't have anything to do with my code--I did run The easiest solution to this problem would be to upgrade |
* Adds support for numpy types specified in dataclasses, converting them to and from standard python types as needed. * Note that this commit does NOT handle the case where a numpy type is misrepresented as a standard python type. In most cases, these values will fail to serialize, or may be serialized incorrectly.
|
Okay, I removed As noted above, I did not change the numpy install conditions, because while your suggestion makes sense, it results in incorrect behavior on Python 3.7. |
|
CI tried to install numpy 1.22 for python 3.7 How about updating the marker condition to use |
|
@kmsquire Sorry for the delay. I don't mind upgrading black! 👍 |
Strange, this was working before. I'll take a look. |
Unfortunately, this doesn't seem to work, nor has anything else I've tried locally. 😢 I'm going to file a bug report with poetry. In the mean time, I'll back out the update to numpy, so we'll be on version 1.21.x for all versions of python. |
|
Getting closer (albeit with a restriction to numpy 1.21 for now). The pypi test image needs to have some blas installed in order to compile numpy. I'm not sure of the best way to do this, and probably won't be able to look into it further until sometime next week. |
|
It's quite hard work to setup build requirements for Windows, Linux and macOS. I don't mind removing |
|
Okay, I think this should finally be good. The latest set of commits
|
|
Closes #145 |
|
@kmsquire Thank you for the great contribution! 🎉👍 |
them to and from standard python types as needed.
Note that this commit does NOT handle the case where a numpytype is misrepresented as a standard python type. In most cases,
these values will fail to serialize, or may be serialized
incorrectly.
The second commit now supports serialization of untyped or incorrectly typed numpy values (i.e., where the type of a variable is a regular Python type, but the contents are a numpy scalar or array) for json and toml only. Deserialization is unaffected in these cases--i.e., the value will be deserialized to the declared (non-numpy) type. (Deserialization to declared numpy types is handled in the first commit).
It should be possible to add support for yaml and toml, but the mechanism for doing so is different and not as obvious.
Closes #145