aeye-lab / pymovements

A python package for processing eye movement data
https://pymovements.readthedocs.io
MIT License
61 stars 12 forks source link

chore!: Drop support for python 3.7 #460

Closed SiQube closed 1 year ago

SiQube commented 1 year ago

resolves #457

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (66278f4) 100.00% compared to head (809f21d) 100.00%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #460 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 55 55 Lines 2152 2145 -7 Branches 503 503 ========================================= - Hits 2152 2145 -7 ``` | [Files Changed](https://app.codecov.io/gh/aeye-lab/pymovements/pull/460?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aeye-lab) | Coverage Δ | | |---|---|---| | [src/pymovements/events/detection/idt.py](https://app.codecov.io/gh/aeye-lab/pymovements/pull/460?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aeye-lab#diff-c3JjL3B5bW92ZW1lbnRzL2V2ZW50cy9kZXRlY3Rpb24vaWR0LnB5) | `100.00% <100.00%> (ø)` | | | [src/pymovements/gaze/integration.py](https://app.codecov.io/gh/aeye-lab/pymovements/pull/460?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aeye-lab#diff-c3JjL3B5bW92ZW1lbnRzL2dhemUvaW50ZWdyYXRpb24ucHk=) | `100.00% <100.00%> (ø)` | | | [src/pymovements/plotting/traceplot.py](https://app.codecov.io/gh/aeye-lab/pymovements/pull/460?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aeye-lab#diff-c3JjL3B5bW92ZW1lbnRzL3Bsb3R0aW5nL3RyYWNlcGxvdC5weQ==) | `100.00% <100.00%> (ø)` | | | [src/pymovements/utils/archives.py](https://app.codecov.io/gh/aeye-lab/pymovements/pull/460?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aeye-lab#diff-c3JjL3B5bW92ZW1lbnRzL3V0aWxzL2FyY2hpdmVzLnB5) | `100.00% <100.00%> (ø)` | | | [src/pymovements/utils/parsing.py](https://app.codecov.io/gh/aeye-lab/pymovements/pull/460?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aeye-lab#diff-c3JjL3B5bW92ZW1lbnRzL3V0aWxzL3BhcnNpbmcucHk=) | `100.00% <100.00%> (ø)` | | | [src/pymovements/utils/paths.py](https://app.codecov.io/gh/aeye-lab/pymovements/pull/460?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aeye-lab#diff-c3JjL3B5bW92ZW1lbnRzL3V0aWxzL3BhdGhzLnB5) | `100.00% <100.00%> (ø)` | |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

SiQube commented 1 year ago

I additionally removed mandatory checks for 3.7,

I think the walrus operator make the code less readable but was required for pylint. @dkrako please carefully check if you think we should either disable the walrus, or if it is readable enough.

see here

for the commit in question

dkrako commented 1 year ago

One more thing which I am unsure of: do we still need from __future__ import annotations everywhere?

We have this special pre-commit hook to add it everywhere:

https://github.com/aeye-lab/pymovements/blob/e3ef6c91fd8671f6cba305cd645d7371c319f711/.pre-commit-config.yaml#L21-L27

I'm not sure if this was due to python3.7 issues or if thisissue will remain with python3.8?

SiQube commented 1 year ago

One more thing which I am unsure of: do we still need from __future__ import annotations everywhere?

We have this special pre-commit hook to add it everywhere:

https://github.com/aeye-lab/pymovements/blob/e3ef6c91fd8671f6cba305cd645d7371c319f711/.pre-commit-config.yaml#L21-L27

I'm not sure if this was due to python3.7 issues or if thisissue will remain with python3.8?

This will remain since there are changes from 3.8 => 3.9 and so on, e.g. here are some 3.12 typing changes. so I'd keep it!

dkrako commented 1 year ago

Thanks a lot for all the work!