Make `load_fragments` to be default behavior for `load` by eunice-chan · Pull Request #190 · GazzolaLab/MiV-OS

eunice-chan · 2023-01-31T02:42:23Z

Changes

Update the load function logic to be load_fragments and remove load_fragments function.

Open Questions:

What to test? (Other than make test)
Not sure what the contextmanager decorator does. Do I need to keep it?
Ran into issues running make install, make pre-commit-install, make test, make formatting:

> make install          
poetry lock -n && poetry export --without-hashes > requirements.txt
Updating dependencies
Resolving dependencies... (10898.5s)

Took a long time so I stopped.
For the others, they simply errored out:

> make pre-commit-install
poetry run pre-commit install
Command not found: pre-commit

> make test
poetry run pytest -c pyproject.toml --cov=miv
Traceback (most recent call last):
  File "/Users/eunice/opt/anaconda3/lib/python3.7/site-packages/py/_vendored_packages/iniconfig.py", line 126, in _parseline
    name, value = line.split('=', 1)
ValueError: not enough values to unpack (expected 2, got 1)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/eunice/opt/anaconda3/lib/python3.7/site-packages/py/_vendored_packages/iniconfig.py", line 131, in _parseline
    name, value = line.split(":", 1)
ValueError: not enough values to unpack (expected 2, got 1)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/eunice/opt/anaconda3/bin/pytest", line 11, in <module>
    sys.exit(main())
  File "/Users/eunice/opt/anaconda3/lib/python3.7/site-packages/_pytest/config/__init__.py", line 71, in main
    config = _prepareconfig(args, plugins)
  File "/Users/eunice/opt/anaconda3/lib/python3.7/site-packages/_pytest/config/__init__.py", line 221, in _prepareconfig
    pluginmanager=pluginmanager, args=args
  File "/Users/eunice/opt/anaconda3/lib/python3.7/site-packages/pluggy/hooks.py", line 286, in __call__
    return self._hookexec(self, self.get_hookimpls(), kwargs)
  File "/Users/eunice/opt/anaconda3/lib/python3.7/site-packages/pluggy/manager.py", line 92, in _hookexec
    return self._inner_hookexec(hook, methods, kwargs)
  File "/Users/eunice/opt/anaconda3/lib/python3.7/site-packages/pluggy/manager.py", line 86, in <lambda>
    firstresult=hook.spec.opts.get("firstresult") if hook.spec else False,
  File "/Users/eunice/opt/anaconda3/lib/python3.7/site-packages/pluggy/callers.py", line 203, in _multicall
    gen.send(outcome)
  File "/Users/eunice/opt/anaconda3/lib/python3.7/site-packages/_pytest/helpconfig.py", line 89, in pytest_cmdline_parse
    config = outcome.get_result()
  File "/Users/eunice/opt/anaconda3/lib/python3.7/site-packages/pluggy/callers.py", line 80, in get_result
    raise ex[1].with_traceback(ex[2])
  File "/Users/eunice/opt/anaconda3/lib/python3.7/site-packages/pluggy/callers.py", line 187, in _multicall
    res = hook_impl.function(*args)
  File "/Users/eunice/opt/anaconda3/lib/python3.7/site-packages/_pytest/config/__init__.py", line 736, in pytest_cmdline_parse
    self.parse(args)
  File "/Users/eunice/opt/anaconda3/lib/python3.7/site-packages/_pytest/config/__init__.py", line 943, in parse
    self._preparse(args, addopts=addopts)
  File "/Users/eunice/opt/anaconda3/lib/python3.7/site-packages/_pytest/config/__init__.py", line 878, in _preparse
    self._initini(args)
  File "/Users/eunice/opt/anaconda3/lib/python3.7/site-packages/_pytest/config/__init__.py", line 809, in _initini
    config=self,
  File "/Users/eunice/opt/anaconda3/lib/python3.7/site-packages/_pytest/config/findpaths.py", line 118, in determine_setup
    iniconfig = py.iniconfig.IniConfig(inifile)
  File "/Users/eunice/opt/anaconda3/lib/python3.7/site-packages/py/_vendored_packages/iniconfig.py", line 54, in __init__
    tokens = self._parse(iter(f))
  File "/Users/eunice/opt/anaconda3/lib/python3.7/site-packages/py/_vendored_packages/iniconfig.py", line 83, in _parse
    name, data = self._parseline(line, lineno)
  File "/Users/eunice/opt/anaconda3/lib/python3.7/site-packages/py/_vendored_packages/iniconfig.py", line 133, in _parseline
    self._raise(lineno, 'unexpected line: %r' % line)
  File "/Users/eunice/opt/anaconda3/lib/python3.7/site-packages/py/_vendored_packages/iniconfig.py", line 77, in _raise
    raise ParseError(self.path, lineno, msg)
py._vendored_packages.iniconfig.ParseError: pyproject.toml:29: unexpected line: ']'
make: *** [test] Error 1

> make formatting
poetry run pyupgrade --exit-zero-even-if-changed --py38-plus **/*.py
Command not found: pyupgrade
make: *** [codestyle] Error 1

…nto load_fragments_to_load

skim0119 · 2023-01-31T04:16:14Z

I'll double check the make calls. I think you couldn't install all the dependencies because make install did not resolve all the issues, which is probably why all other commands didn't run.

…b/MiV-OS into load_fragments_to_load

eunice-chan · 2023-02-02T17:29:41Z

@skim0119 I appreciate your help! I am able to run make install and make pre-commit-install now, but encounter the following error when running make test:

____________________________________________________ ERROR collecting tests/coding/test_bsa.py ____________________________________________________
ImportError while importing test module '/Users/eunice/Desktop/MiV-OS/tests/coding/test_bsa.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
../../opt/anaconda3/envs/miv-dev/lib/python3.8/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
tests/coding/test_bsa.py:4: in <module>
    from miv.coding.temporal.spiker import BensSpikerAlgorithm
miv/coding/temporal/__init__.py:11: in <module>
    from miv.coding.temporal.lyon import *
miv/coding/temporal/lyon.py:11: in <module>
    import lyon.calc
E   ModuleNotFoundError: No module named 'lyon'
_________________________________________________ ERROR collecting tests/coding/test_ear_model.py _________________________________________________
ImportError while importing test module '/Users/eunice/Desktop/MiV-OS/tests/coding/test_ear_model.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
../../opt/anaconda3/envs/miv-dev/lib/python3.8/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
tests/coding/test_ear_model.py:4: in <module>
    from miv.coding.temporal import LyonEarModel
miv/coding/temporal/__init__.py:11: in <module>
    from miv.coding.temporal.lyon import *
miv/coding/temporal/lyon.py:11: in <module>
    import lyon.calc
E   ModuleNotFoundError: No module named 'lyon'
_____________________________________________ ERROR collecting tests/io/serial/test_arduino_ports.py ______________________________________________
ImportError while importing test module '/Users/eunice/Desktop/MiV-OS/tests/io/serial/test_arduino_ports.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
../../opt/anaconda3/envs/miv-dev/lib/python3.8/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
tests/io/serial/test_arduino_ports.py:6: in <module>
    from miv.io.serial import ArduinoSerial, list_serial_ports
miv/io/serial/__init__.py:1: in <module>
    from miv.io.serial.arduino import *
miv/io/serial/arduino.py:14: in <module>
    import serial
E   ModuleNotFoundError: No module named 'serial'
________________________________________________ ERROR collecting tests/io/serial/test_stimjim.py _________________________________________________
ImportError while importing test module '/Users/eunice/Desktop/MiV-OS/tests/io/serial/test_stimjim.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
../../opt/anaconda3/envs/miv-dev/lib/python3.8/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
tests/io/serial/test_stimjim.py:4: in <module>
    from miv.io.serial import ArduinoSerial, StimjimSerial
miv/io/serial/__init__.py:1: in <module>
    from miv.io.serial.arduino import *
miv/io/serial/arduino.py:14: in <module>
    import serial
E   ModuleNotFoundError: No module named 'serial'

skim0119 · 2023-02-02T17:35:11Z

@eunice-chan There are some extras installation for full test. Try to run test after poetry install --all-extras

eunice-chan · 2023-02-02T18:11:53Z

@skim0119 Thanks! I was able to successfuly run make test. I don't think poetry install --all-extras is currently mentioned in the CONTRIBUTING.md file. We may want to add it in.

This branch has some conflicts with main in the poetry.lock file. How should I resolve them? Should I just accept the changes from main? Once that is done, I think I am ready to merge this into update-0.2.4 :)

skim0119

Some minor change, and everything looks good

skim0119 · 2023-02-02T19:18:32Z

@eunice-chan I'll resolve the lock file conflict at last before merging. It shouldn't happen usually, but happens because this PR contains some dependency update.

skim0119 · 2023-02-02T19:22:18Z

@skim0119 Thanks! I was able to successfuly run make test. I don't think poetry install --all-extras is currently mentioned in the CONTRIBUTING.md file. We may want to add it in.

Thanks. I just added an extra description a8499fe.

Co-authored-by: Seung Hyun Kim <skim0119@gmail.com>

…b/MiV-OS into load_fragments_to_load

codecov-commenter · 2023-02-02T21:45:55Z

Codecov Report

Base: 74.53% // Head: 75.94% // Increases project coverage by +1.41% 🎉

Coverage data is based on head (d92fed4) compared to base (6b41458).
Patch coverage: 84.44% of modified lines in pull request are covered.

❗ Current head d92fed4 differs from pull request most recent head 6b4ea00. Consider uploading reports for the commit 6b4ea00 to get more accurate results

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files

@@               Coverage Diff                @@
##           update-0.2.4     #190      +/-   ##
================================================
+ Coverage         74.53%   75.94%   +1.41%     
================================================
  Files                55       56       +1     
  Lines              1920     1937      +17     
  Branches            280      284       +4     
================================================
+ Hits               1431     1471      +40     
+ Misses              399      376      -23     
  Partials             90       90

Flag	Coverage Δ
unittests	`75.94% <84.44%> (+1.41%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
miv/io/binary.py	`83.33% <ø> (+0.85%)`	⬆️
miv/io/intan/data.py	`0.00% <ø> (ø)`
miv/signal/similarity/simple.py	`83.33% <83.33%> (ø)`
miv/io/data.py	`76.59% <100.00%> (+0.45%)`	⬆️
miv/io/protocol.py	`75.00% <100.00%> (+2.77%)`	⬆️
miv/signal/similarity/__init__.py	`100.00% <100.00%> (+100.00%)`	⬆️
miv/signal/similarity/dtw.py	`57.14% <0.00%> (+57.14%)`	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

eunice-chan · 2023-02-02T21:58:53Z

@skim0119 I think I merged with main as well as update-0.2.4 and as a result, see 11 files changed in this PR. How do I resolve this?

skim0119 · 2023-02-02T22:03:45Z

@skim0119 I think I merged with main as well as update-0.2.4 and as a result, see 11 files changed in this PR. How do I resolve this?

@eunice-chan Its fine. I'll merge main->update-0.2.4 which should resolve things.

skim0119 · 2023-02-02T22:09:00Z

I'll fix the test case, I know the issue.

…b/MiV-OS into load_fragments_to_load

skim0119 · 2023-02-03T04:12:14Z

@eunice-chan FYI, contextmanager was needed because I initially intended to use load method with with data.load() as ... syntax. Since we are changing its behavior to be a generator, we don't need it anymore.

…b/MiV-OS into load_fragments_to_load

eunice-chan · 2023-02-03T21:20:13Z

@skim0119 I seem to have removed too many contextmanagers and am failing some tests. Can you look at it and let me know what is the most I can remove?

skim0119 · 2023-02-03T22:27:58Z

@eunice-chan Its actually good that you went through the codebase and removed them. Some other modules and tests were still using the old syntax of load(), and that is what caused the CI to fail. I went through them just now and fixed them. (4c43504)

skim0119

@eunice-chan Looks good to me. Thanks for the PR

eunice-chan added 2 commits January 30, 2023 14:44

Load_fragments -> Load

fe1447a

Merge branch 'update-0.2.4' of https://github.com/GazzolaLab/MiV-OS i…

4910f55

…nto load_fragments_to_load

eunice-chan requested a review from skim0119 January 31, 2023 02:42

skim0119 reviewed Jan 31, 2023

View reviewed changes

Comment thread docs/guide/read_data_fragment.md Outdated

skim0119 assigned eunice-chan Jan 31, 2023

skim0119 added documentation Improvements or additions to documentation enhancement New feature or request labels Jan 31, 2023

skim0119 added 2 commits January 31, 2023 13:51

update lock file

e659de2

update makefile

bd09566

skim0119 changed the title ~~Load fragments --> load~~ [WIP] Make load_fragments to be default behavior for load Jan 31, 2023

skim0119 and others added 4 commits January 31, 2023 18:12

Merge branch 'update-0.2.4' into load_fragments_to_load

d47d08a

Merge main

2f81970

Merge branch 'load_fragments_to_load' of https://github.com/GazzolaLa…

861658c

…b/MiV-OS into load_fragments_to_load

Update documentation

1870e9a

skim0119 requested changes Feb 2, 2023

View reviewed changes

Comment thread docs/guide/read_data_fragment.md Outdated

Comment thread docs/tutorial/signal_processing.md

skim0119 marked this pull request as ready for review February 2, 2023 19:18

skim0119 changed the title ~~[WIP] Make load_fragments to be default behavior for load~~ Make load_fragments to be default behavior for load Feb 2, 2023

eunice-chan and others added 3 commits February 2, 2023 13:37

Update docs/tutorial/signal_processing.md

c9255e2

Co-authored-by: Seung Hyun Kim <skim0119@gmail.com>

Move documentation

f00a8b7

Merge branch 'load_fragments_to_load' of https://github.com/GazzolaLa…

768de3a

…b/MiV-OS into load_fragments_to_load

Merge branch 'update-0.2.4' into load_fragments_to_load

80fb7d0

Merge branch 'update-0.2.4' into load_fragments_to_load

b672e48

skim0119 and others added 4 commits February 2, 2023 16:17

Merge branch 'update-0.2.4' into load_fragments_to_load

01a8197

Merge branch 'update-0.2.4' into load_fragments_to_load

6114181

Merge branch 'update-0.2.4' into load_fragments_to_load

dac76fe

Merge branch 'load_fragments_to_load' of https://github.com/GazzolaLa…

f5384fd

…b/MiV-OS into load_fragments_to_load

remove context manager

bfc47c0

skim0119 force-pushed the load_fragments_to_load branch from f1e9623 to bfc47c0 Compare February 3, 2023 04:13

eunice-chan added 4 commits February 3, 2023 14:50

Remove read_data_fragment from guide/index.rst

19c0c5b

Merge branch 'load_fragments_to_load' of https://github.com/GazzolaLa…

d92fed4

…b/MiV-OS into load_fragments_to_load

Remove contextmanager

6b4ea00

Remove contextmanager import

ffb0795

eunice-chan commented Feb 3, 2023

View reviewed changes

Comment thread Makefile

Comment thread miv/io/data.py

Comment thread tests/io/mock_data.py

update: fix unittests and other modules using contextmanager->generator

4c43504

skim0119 approved these changes Feb 3, 2023

View reviewed changes

skim0119 merged commit 47e587a into update-0.2.4 Feb 3, 2023

skim0119 deleted the load_fragments_to_load branch February 3, 2023 22:28

Conversation

eunice-chan commented Jan 31, 2023 • edited by skim0119 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Open Questions:

Uh oh!

Uh oh!

skim0119 commented Jan 31, 2023

Uh oh!

eunice-chan commented Feb 2, 2023

Uh oh!

skim0119 commented Feb 2, 2023

Uh oh!

eunice-chan commented Feb 2, 2023

Uh oh!

skim0119 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

skim0119 commented Feb 2, 2023

Uh oh!

skim0119 commented Feb 2, 2023

Uh oh!

codecov-commenter commented Feb 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

eunice-chan commented Feb 2, 2023

Uh oh!

skim0119 commented Feb 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

skim0119 commented Feb 2, 2023

Uh oh!

skim0119 commented Feb 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

eunice-chan commented Feb 3, 2023

Uh oh!

skim0119 commented Feb 3, 2023

Uh oh!

skim0119 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

eunice-chan commented Jan 31, 2023 •

edited by skim0119

Loading

codecov-commenter commented Feb 2, 2023 •

edited

Loading

skim0119 commented Feb 2, 2023 •

edited

Loading

skim0119 commented Feb 3, 2023 •

edited

Loading