Fix exports #2584

catalintoma · 2022-09-21T17:56:13Z

Description

Fix Eclect.us and Finviz not allowing exports

How has this been tested?

Refactored unit tests to take into account exports

DidierRLopes · 2022-09-21T17:58:10Z

Thanks for this @catalintoma 🙏🏽

catalintoma · 2022-09-21T18:01:21Z

@DidierRLopes have mercy, this is the first time I've written a line of Python code but I really like it.
Also, this terminal is awesome

I am using WSL for development and did not succeed in installing the pre-commit hooks. If I understand what failed in linting I will fix manually, or request some help to install the hooks.

DidierRLopes · 2022-09-21T18:08:33Z

I will! This is great! We are actually working on improving substantially our CONTRIBUTING guidelines to make it easier for contributors to understand the code architecture (see here), it's great that you were able to even go into the tests!!

Thanks for feedback. We are looking to work on some content in video format regarding leveraging all capabilities of terminal :)

Hummm, we will have the team look into this (cough @colin99d cough 😄). In this case the black linter failed, so you can do:

black path_to_file

in order to fix it.

catalintoma · 2022-09-21T19:05:10Z

@DidierRLopes I ran Black on the files reported in the failed job.
I first tried

black --diff --check .

But it reported it would reformat 445 files, so that's why I did it manually for the 5 files.

I will also search for a VSCode extension for Black formatter.
I don't think I've ever had so many things I am unfamiliar with at the same time, but I like a challenge.

Currently I develop on VS Code installed on Windows and a "Remote - WSL" extension that allows to develop in WSL.

catalintoma · 2022-09-21T19:09:34Z

@DidierRLopes
I don't want to pollute this PR, but this is the error I get after installing pre-commit hook:

2022-09-21T19:08:17.526Z] [INFO] Installing environment for local.
[INFO] Once installed this environment will be reused.
[INFO] This may take a few minutes...
An unexpected error has occurred: CalledProcessError: command: ('/home/catalin/anaconda3/envs/obb/bin/python3.9', '-mvirtualenv', '/home/catalin/.cache/pre-commit/repo62qim38a/py_env-python3.9')
return code: 1
expected return code: 0
stdout:
    ModuleNotFoundError: No module named 'distlib'
    
stderr: (none)

DidierRLopes · 2022-09-21T22:51:39Z

@DidierRLopes I ran Black on the files reported in the failed job. I first tried

black --diff --check .

But it reported it would reformat 445 files, so that's why I did it manually for the 5 files.

I will also search for a VSCode extension for Black formatter. I don't think I've ever had so many things I am unfamiliar with at the same time, but I like a challenge.

Currently I develop on VS Code installed on Windows and a "Remote - WSL" extension that allows to develop in WSL.

That's exactly the same I do, using black manually. Well actually if the pre-commit hook is working, it does that for you.

Let's see if the tests pass now.

Have never seen pre-commit install returning that. @colin99d have you?

colin99d · 2022-09-22T05:51:16Z

@catalintoma, thanks for the pr! Did you setup your environment using the following steps:

Install miniconda: https://conda.io/projects/conda/en/latest/user-guide/install/windows.html
Clone the repo: git clone https://github.com/OpenBB-finance/OpenBBTerminal.git
Enter the directory: cd OpenBBTerminal/
Create an environment: conda env create -n obb --file build/conda/conda-3-9-env.yaml
Activate and environment: conda activate obb
Install dependencies poetry install

catalintoma · 2022-09-22T06:54:10Z

@colin99d yes, the only difference is that I installed the full Anaconda3 and I have Debian in my WSL, so I've installed it from this guide:
https://docs.anaconda.com/anaconda/install/linux/

I did now just push formatting on 2 files that I've already formatted.

The weird thing is that if I try to format using black from the terminal it does nothing.
But if I use the Black VS Code extension + format on save, it produced the correct formatting (I've looked at the logs from the "General Linting" workflow).

colin99d · 2022-09-22T09:13:41Z

That is really interesting. Unfortunately, I do not have windows or linux so I cannot test this. I typically just run black .. Does that work for you?

catalintoma · 2022-09-22T10:43:59Z

That is really interesting. Unfortunately, I do not have windows or linux so I cannot test this. I typically just run black .. Does that work for you?

black .
does work, now when I run it it does not change anything, so that should be good.

I did push another fix for an error generated by flake8 (it looks to be a static code analyzer). Can you run the pipelines again please?

I think running black / flake8 (who knows what else) manually is a big waste of time, seems like having the pre-commit hook is mandatory.

colin99d · 2022-09-22T10:52:41Z

Yup all of these are just checking you code to make sure it meets some basic guidelines.

colin99d · 2022-09-22T10:52:58Z

Try running pip install distlib. That might fix your issue.

catalintoma · 2022-09-22T12:22:14Z

Now it was time for pylint to fail, I pushed a fix for that also, can we try a deploy again?
I like how pylint tells me (after it shows errors)
Your code has been rated at 10.00/10 :D

Try running pip install distlib. That might fix your issue.

I've already tried installing 'distlib' but it returns this error
Requirement already satisfied: distlib in /home/catalin/anaconda3/envs/obb/lib/python3.9/site-packages (0.3.6)

I did a search for that module and I can see it in the poetry.lock file

colin99d · 2022-09-22T12:24:38Z

Hmmm. And pip install pre-commit is still not working?

catalintoma · 2022-09-22T12:34:32Z

Hmmm. And pip install pre-commit is still not working?

Well pre-commit is already installed, it's after I run
pre-commit install

Than I can't commit and get this error in Git output
[2022-09-22T12:31:47.591Z] [INFO] Installing environment for local. [INFO] Once installed this environment will be reused. [INFO] This may take a few minutes... An unexpected error has occurred: CalledProcessError: command: ('/home/catalin/anaconda3/envs/obb/bin/python3.9', '-mvirtualenv', '/home/catalin/.cache/pre-commit/repo62qim38a/py_env-python3.9') return code: 1 expected return code: 0 stdout: ModuleNotFoundError: No module named 'distlib'

There is also a pre-commit.log file created that contains this:
`

version information

pre-commit version: 2.20.0
git --version: git version 2.34.1
sys.version:
    3.9.13 | packaged by conda-forge | (main, May 27 2022, 16:56:21) 
    [GCC 10.3.0]
sys.executable: /home/catalin/anaconda3/envs/obb/bin/python3.9
os.name: posix
sys.platform: linux

error information

An unexpected error has occurred: CalledProcessError: command: ('/home/catalin/anaconda3/envs/obb/bin/python3.9', '-mvirtualenv', '/home/catalin/.cache/pre-commit/repo62qim38a/py_env-python3.9')
return code: 1
expected return code: 0
stdout:
    ModuleNotFoundError: No module named 'distlib'
    
stderr: (none)

Traceback (most recent call last):
  File "/home/catalin/anaconda3/envs/obb/lib/python3.9/site-packages/pre_commit/error_handler.py", line 73, in error_handler
    yield
  File "/home/catalin/anaconda3/envs/obb/lib/python3.9/site-packages/pre_commit/main.py", line 358, in main
    return hook_impl(
  File "/home/catalin/anaconda3/envs/obb/lib/python3.9/site-packages/pre_commit/commands/hook_impl.py", line 254, in hook_impl
    return retv | run(config, store, ns)
  File "/home/catalin/anaconda3/envs/obb/lib/python3.9/site-packages/pre_commit/commands/run.py", line 424, in run
    install_hook_envs(to_install, store)
  File "/home/catalin/anaconda3/envs/obb/lib/python3.9/site-packages/pre_commit/repository.py", line 223, in install_hook_envs
    _hook_install(hook)
  File "/home/catalin/anaconda3/envs/obb/lib/python3.9/site-packages/pre_commit/repository.py", line 79, in _hook_install
    lang.install_environment(
  File "/home/catalin/anaconda3/envs/obb/lib/python3.9/site-packages/pre_commit/languages/python.py", line 219, in install_environment
    cmd_output_b(*venv_cmd, cwd='/')
  File "/home/catalin/anaconda3/envs/obb/lib/python3.9/site-packages/pre_commit/util.py", line 146, in cmd_output_b
    raise CalledProcessError(returncode, cmd, retcode, stdout_b, stderr_b)
pre_commit.util.CalledProcessError: command: ('/home/catalin/anaconda3/envs/obb/bin/python3.9', '-mvirtualenv', '/home/catalin/.cache/pre-commit/repo62qim38a/py_env-python3.9')
return code: 1
expected return code: 0
stdout:
    ModuleNotFoundError: No module named 'distlib'
    
stderr: (none)

`

catalintoma · 2022-09-23T18:24:44Z

@DidierRLopes @colin99d I did pull from main again and it seems it's waiting approval for 2 workflows.

I think they will pass because they passed last time.

What is the process of pushing this to main? Do we wait for a release train?

DidierRLopes · 2022-09-23T19:27:16Z

@DidierRLopes @colin99d I did pull from main again and it seems it's waiting approval for 2 workflows.

I think they will pass because they passed last time.

What is the process of pushing this to main? Do we wait for a release train?

Sorry, I've had a very hectic day! Just approved to run tests, if it passes we can merge it to main 🙏🏽

catalintoma · 2022-09-23T19:58:46Z

Sorry, I've had a very hectic day! Just approved to run tests, if it passes we can merge it to main 🙏🏽

No worries, it's no rush, I was just anxious to get my first fix in.

I am eyeing this bug next:
#2583

DidierRLopes · 2022-09-23T20:54:24Z

Sorry, I've had a very hectic day! Just approved to run tests, if it passes we can merge it to main 🙏🏽

No worries, it's no rush, I was just anxious to get my first fix in.

I am eyeing this bug next: #2583

That would be great! I just updated this branch with main so we should be able to merge it after 😄

colin99d · 2022-09-23T21:18:20Z

Congrats on becoming a contributor @catalintoma. This PR looks great!

catalintoma · 2022-09-23T21:46:45Z

Congrats on becoming a contributor @catalintoma. This PR looks great!

Thank you!

DidierRLopes · 2022-09-23T23:39:55Z

Welcome! 🥂

catalintoma added 2 commits September 21, 2022 20:39

Fix exports missing on Eclect.US and Finviz

a16a693

Merge branch 'OpenBB-finance:main' into fix-exports

7713896

DidierRLopes added the bug Fix bug label Sep 21, 2022

Fix formatting

87af054

Fix formatting

dd53bfe

Fix lamda not using 'def'

9fc0efd

Fix pylint issues

470afb1

Merge branch 'main' into fix-exports

5c3f9a0

catalintoma closed this Sep 22, 2022

catalintoma reopened this Sep 22, 2022

catalintoma added 2 commits September 22, 2022 20:28

Merge branch 'OpenBB-finance:main' into fix-exports

a82252a

Merge branch 'main' into fix-exports

df6c1d1

Merge branch 'main' into fix-exports

454d3b1

colin99d self-requested a review September 23, 2022 21:17

colin99d approved these changes Sep 23, 2022

View reviewed changes

colin99d merged commit d0914a8 into OpenBB-finance:main Sep 23, 2022

catalintoma deleted the fix-exports branch September 24, 2022 07:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix exports #2584

Fix exports #2584

catalintoma commented Sep 21, 2022

DidierRLopes commented Sep 21, 2022

catalintoma commented Sep 21, 2022 •

edited

Loading

DidierRLopes commented Sep 21, 2022

catalintoma commented Sep 21, 2022

catalintoma commented Sep 21, 2022 •

edited

Loading

DidierRLopes commented Sep 21, 2022

colin99d commented Sep 22, 2022

catalintoma commented Sep 22, 2022

colin99d commented Sep 22, 2022

catalintoma commented Sep 22, 2022

colin99d commented Sep 22, 2022

colin99d commented Sep 22, 2022

catalintoma commented Sep 22, 2022 •

edited

Loading

colin99d commented Sep 22, 2022

catalintoma commented Sep 22, 2022

catalintoma commented Sep 23, 2022

DidierRLopes commented Sep 23, 2022

catalintoma commented Sep 23, 2022

DidierRLopes commented Sep 23, 2022

colin99d commented Sep 23, 2022

catalintoma commented Sep 23, 2022

DidierRLopes commented Sep 23, 2022

Fix exports #2584

Fix exports #2584

Conversation

catalintoma commented Sep 21, 2022

Description

How has this been tested?

DidierRLopes commented Sep 21, 2022

catalintoma commented Sep 21, 2022 • edited Loading

DidierRLopes commented Sep 21, 2022

catalintoma commented Sep 21, 2022

catalintoma commented Sep 21, 2022 • edited Loading

DidierRLopes commented Sep 21, 2022

colin99d commented Sep 22, 2022

catalintoma commented Sep 22, 2022

colin99d commented Sep 22, 2022

catalintoma commented Sep 22, 2022

colin99d commented Sep 22, 2022

colin99d commented Sep 22, 2022

catalintoma commented Sep 22, 2022 • edited Loading

colin99d commented Sep 22, 2022

catalintoma commented Sep 22, 2022

version information

error information

catalintoma commented Sep 23, 2022

DidierRLopes commented Sep 23, 2022

catalintoma commented Sep 23, 2022

DidierRLopes commented Sep 23, 2022

colin99d commented Sep 23, 2022

catalintoma commented Sep 23, 2022

DidierRLopes commented Sep 23, 2022

catalintoma commented Sep 21, 2022 •

edited

Loading

catalintoma commented Sep 21, 2022 •

edited

Loading

catalintoma commented Sep 22, 2022 •

edited

Loading