Ozon3Org / Ozon3

An open-source Python package to easily obtain real-time, historical, or forecasted air quality data for anywhere in the world. Reliable, accurate and simple.
GNU General Public License v3.0
66 stars 23 forks source link

Ozone doesn't import, 'ModuleNotFoundError: No module named 'ozone.historical'' #119

Closed Milind220 closed 2 years ago

Milind220 commented 2 years ago

Here is the complete error message when I try to import Ozone while using it as a user would.

Jupyter Notebook

Screenshot 2022-04-20 at 4 58 17 PM

Python file

Screenshot 2022-04-20 at 4 58 00 PM

It sounds like it has something to do with the relative import.

isi1000 commented 2 years ago

I'm having this issue, how can I fix it?

Milind220 commented 2 years ago

@isi1000 Hey! Still trying to figure out what exactly is going wrong with that import, but we'll have it fixed soon!

If you simply want to use the rest of Ozone's features (non historical data features), you can try using Ozone v1.5.5:

  1. pip uninstall ozon3
  2. pip install ozon3==1.5.5

If you'd like to work on it and contribute to Ozone, then that's great too. I'd experiment with the import to try and understand why it's behaving that way.

Milind220 commented 2 years ago

@lahdjirayhan I think I've managed to solve this quite simply. The error appeared to be indicating that it couldn't find the module. By adding an init.py in the historical directory, it turns it into a local package, and the error goes away. I'll confirm this with a testPyPI package upload to check if it has actually worked.

EDIT: It didn't work... kinda?

When I locally import the package (in a virtual env where ozone hasn't been pip installed), it does work. However, After uploading to testPyPI as a package here, and then installing that, It throws a different error. Shown below:

Screenshot 2022-04-21 at 1 19 20 AM
lahdjirayhan commented 2 years ago

I didn't see that coming. @Milind220


As for giving __init__.py, maybe you're on the right track. The Python module/package distinction confuses me, so that might slip past me before.


The last image you sent indicates that relevant_funcs.js (where the relevant part of JS script lives) isn't present in the Ozone installation. My un-educated guess: this JS file is not bundled/packaged and/or installed/copied properly when using installation from PyPI (note to self: we develop using local editable mode, so we miss it). Please confirm this if you can, while you're at it @Milind220. Maybe it's the problem.

If it is, one hackish workaround would be just taking the content of relevant_funcs.js into a long triple-quoted string within a Python file, to make sure it is packaged onto PyPI.

The _JS_FUNCS variable in _reverse_engineering.py is just a long string, after all.


EDIT: I've tried installing from test-PyPI in a new separate venv. I can replicate your error @Milind220. When I manually copy-pasted a relevant_funcs.js file onto the installation folder (Ozone-test\venv\Lib\site-packages\ozone\historical), the error goes away. Seems to reinforce my guess earlier about the .js file not bundled/packaged.

isi1000 commented 2 years ago

@lahdjirayhan @Milind220 Indeed, all the historical folder doesn't download when you use pip, but I added it manually, and there is another problem with _reverse_engineering.py and relevant_funcs.js.
image

(EDIT)

I sort out the problem by just coping in ozone.py all the _reverse_engineered.py code and changing the location of the relevant_funcs.js archive to the same folder.

Milind220 commented 2 years ago

Please confirm this if you can, while you're at it @Milind220. Maybe it's the problem.

Yeah I'll check it out too, though that would make sense!

If it is, one hackish workaround would be just taking the content of relevant_funcs.js into a long triple-quoted string within a Python file, to make sure it is packaged onto PyPI.

The _JS_FUNCS variable in _reverse_engineering.py is just a long string, after all.

Hahaha this would work, and that's the main thing. We can go with this.

EDIT: I've tried installing from test-PyPI in a new separate venv. I can replicate your error @Milind220. When I manually copy-pasted a relevant_funcs.js file onto the installation folder (Ozone-test\venv\Lib\site-packages\ozone\historical), the error goes away. Seems to reinforce my guess earlier about the .js file not bundled/packaged.

Great! I'll take a look now too :)

EDIT

As for giving init.py, maybe you're on the right track. The Python module/package distinction confuses me, so that might slip past me before.

Was the init.py needed at all? or was it just the missing .js file that was wreaking havoc?

EDIT 2

Okay, so it's just the relevant_funcs.js file that was required, not the init.py. Awesome!

Milind220 commented 2 years ago

@isi1000 Great, seems to confirm what @lahdjirayhan was thinking too!

Milind220 commented 2 years ago

I could either fix this myself

OR 👀

@isi1000 Would you like to try fixing this and contributing to Ozone? This is a pretty simple little fix by the looks of it. No worries if you don't want to, of course :)

EDIT:

To suggest up what has to be done

lahdjirayhan commented 2 years ago

Awesome, everyone!


Was the init.py needed at all? or was it just the missing .js file that was wreaking havoc? Okay, so it's just the relevant_funcs.js file that was required, not the init.py. Awesome!

I really don't know with regards to init. But judging from the initial error you found: ModuleNotFoundError: No module named 'ozone.historical', it's best to include the init file. Just in case. @Milind220

Milind220 commented 2 years ago

I'd like to get this fixed asap, so someone hold my beer, I got this

Milind220 commented 2 years ago

Fixed this with 0e289df

lahdjirayhan commented 2 years ago

I can confirm that the new hotfix solves this issue on my machine.

Milind220 commented 2 years ago

That's great!

You may notice that the commits in the hotfix didn't pass the pre-commit/pre merge lints. The relevant_funcs.py file kept failing flake8 formatting criteria (lines were 'too long'), and black didn't seem to reformat it at all. At that point I just committed with --no-verify to avoid triggering the pre commit hooks.

lahdjirayhan commented 2 years ago

I would say it's safe to just add # flake8: noqa to the top of the file. Being just a really long string.

Milind220 commented 2 years ago

Excellent, I'll add that in