Draft: Nimporter v2.0.0rc

[Pebaz]

It's time to figure out what needs to be done to merge this.

Nimporter 2.0 has been tested by a few projects successfully but there will undoubtedly be issues that arise.

Some pre-release boxes to check:

• [ ] Upload to Pypi
• [x] Check on CI
• [ ] Perhaps document what platforms are supported but make a note to increase them

[SekouDiaoNlp]

Hi @Pebaz , what's up?

I have pretty busy lately and couldn't really commit time to the project.

Fortunately I have a month of holidays starting yesterday so it's the perfect timing for me to contribute again.

[SekouDiaoNlp]

I'll start by checking why some CI jobs fail.

[Pebaz]

Hi @SekouDiaoNlp ! Awesome, thanks 👍 The changes in this PR are a bit old but I thought the pipeline was running last time I checked. I'm hoping to get 2.0 merged and be done with it lol 😆

[SekouDiaoNlp]

Hey @Pebaz , I am back from holidays and am free for the rest of the month.

What areas do you want me to focus on for the review?

Cheers and happy new year 🎉

[Pebaz]

Hi @SekouDiaoNlp, Happy new year to you as well!

Basically I'm not really able to work on Nimporter very much anymore. If you could get the pipeline passing so it can be merged that would be amazing.

One area that is pretty important is the bundling feature of v2. It compiles a matrix of ARCHxOS number of source distributions in C and then selects the correct one when the user initiates the package install. I'm pretty certain this will be a big breaking change with a loss of capability but it was definitely the best decision for the future of Nimporter because it solidified the concepts in v2 rather than being too open.

Since v1 will always be available on Pypi, I would be happy with just getting this merged so my giant refactor isn't wasted but I don't have the time to fix the smaller issues with the pipeline.

Here are some other things:

• pyproject.toml update
• Adding more platforms to the platform and architecture tables

Oh one thing that will probably be helpful is the updated docs: https://github.com/Pebaz/nimporter/tree/809a8efc7ca43cb62916f53f37bc0e5294c7bcf4

I put a lot of effort into explaining everything to reduce GitHub issues from users, etc. Hopefully it will help the transition to v2 for everyone.

Let me know if I can answer any questions, thanks a ton for looking into this! 😃

[SekouDiaoNlp]

Hi @SekouDiaoNlp! Basically I'm not really able to work on Nimporter very much anymore. If you could get the pipeline passing so it can be merged that would be amazing.

One area that is pretty important is the bundling feature of v2. It compiles a matrix of ARCHxOS number of source distributions in C and then selects the correct one when the user initiates the package install. I'm pretty certain this will be a big breaking change with a loss of capability but it was definitely the best decision for the future of Nimporter because it solidified the concepts in v2 rather than being too open.

Since v1 will always be available on Pypi, I would be happy with just getting this merged so my giant refactor isn't wasted but I don't have the time to fix the smaller issues with the pipeline.

Here are some other things:

• pyproject.toml update
• Adding more platforms to the platform and architecture tables

Oh one thing that will probably be helpful is the updated docs: https://github.com/Pebaz/nimporter/tree/809a8efc7ca43cb62916f53f37bc0e5294c7bcf4

I put a lot of effort into explaining everything to reduce GitHub issues from users, etc. Hopefully it will help the transition to v2 for everyone.

Let me know if I can answer any questions, thanks a ton for looking into this! 😃

Hey @Pebaz .

I will take time to thoroughly read the updated documentation. I will then go ahead and fix the pipeline, update the pyproject.py and make changes to the platform and architecture table. If I see anything else that needs tidying up I'll let you know.

Cheers

[SekouDiaoNlp]

Hi @Pebaz ,

just a quick update.

I have read the new docs and started going through the changes in the code. I am impressed by the breadth of the refactor, and the new design is much cleaner and hassle free for the end user. The new documentation is also pretty slick.

I’ll start reviewing the code tomorrow and keep you updated on my progress.

Amazing work mate 🤙🏿

[SekouDiaoNlp]

Hi @Pebaz .

i have been looking at the logs of the last CI build.

It seems like building wheels on Windows 10 and MacOs is the culprit for most of the failed builds. I am re-running the failed builds with debug logging to investigate further.

I’ll keep you updated.

Cheers.

[SekouDiaoNlp]

The error for the task https://github.com/Pebaz/nimporter/actions/runs/3717032410/jobs/6643373074

C:\hostedtoolcache\windows\Python\3.10.9\x64\lib\site-packages\coverage\report.py:114:
CoverageWarning: Couldn't parse 'C:\Users\runneradmin\AppData\Local\Temp\pip-req-build-ixa0qjhh\py_module.py':
No source for code: 'C:\Users\runneradmin\AppData\Local\Temp\pip-req-build-ixa0qjhh\py_module.py'. (couldnt-parse)

The error for the task https://github.com/Pebaz/nimporter/actions/runs/3717032410/jobs/6643373074

@contextlib.contextmanager
[4651](https://github.com/Pebaz/nimporter/actions/runs/3717032410/jobs/6643373074#step:6:4652)
    def temporarily_install_nimporter():
[4652](https://github.com/Pebaz/nimporter/actions/runs/3717032410/jobs/6643373074#step:6:4653)
        try:
[4653](https://github.com/Pebaz/nimporter/actions/runs/3717032410/jobs/6643373074#step:6:4654)
            code, _, _ = run_process(
[4654](https://github.com/Pebaz/nimporter/actions/runs/3717032410/jobs/6643373074#step:6:4655)
                shlex.split(f'{PYTHON} setup.py install --force'),
[4655](https://github.com/Pebaz/nimporter/actions/runs/3717032410/jobs/6643373074#step:6:4656)
                'NIMPORTER_INSTRUMENT' in os.environ
[4656](https://github.com/Pebaz/nimporter/actions/runs/3717032410/jobs/6643373074#step:6:4657)
            )
[4657](https://github.com/Pebaz/nimporter/actions/runs/3717032410/jobs/6643373074#step:6:4658)

[4658](https://github.com/Pebaz/nimporter/actions/runs/3717032410/jobs/6643373074#step:6:4659)
>           assert code == 0, 'Nimporter failed to install'
[4659](https://github.com/Pebaz/nimporter/actions/runs/3717032410/jobs/6643373074#step:6:4660)
E           AssertionError: Nimporter failed to install

The error for the task https://github.com/Pebaz/nimporter/actions/runs/3717032413/jobs/6643387261

Poetry could not find a pyproject.toml file in /Users/runner/work/nimporter/nimporter or its parents

The error for the task https://github.com/Pebaz/nimporter/actions/runs/3717032413/jobs/6643387391

Poetry could not find a pyproject.toml file in /Users/runner/work/nimporter/nimporter or its parents

The pyoroject.toml errors will be pretty easy to fix. I will start by those errors and investigate the remainder later.

[SekouDiaoNlp]

I will look into the github action issues later today or tomorrow.

[Pebaz]

Fantastic thank you so much!

[SekouDiaoNlp]

Hey @Pebaz , I fixed and updated the workflow files to the latest supported versions of Python, Nim and the github actions.

Now all tests work on all platforms of the matrix except 1 specific test, test_bdist_wheel_all_targets_installs_correctly which fails on windows + python 3.10 and 3.11.

This is the error message :

C:\hostedtoolcache\windows\Python\3.10.9\x64\lib\site-packages\coverage\report.py:114:
CoverageWarning: Couldn't parse 'C:\Users\runneradmin\AppData\Local\Temp\pip-req-build-fn4yhtcb\py_module.py':
No source for code: 'C:\Users\runneradmin\AppData\Local\Temp\pip-req-build-fn4yhtcb\py_module.py'. (couldnt-parse)

https://github.com/Pebaz/nimporter/actions/runs/3961961676/jobs/6788029105

https://github.com/Pebaz/nimporter/actions/runs/3961961676/jobs/6788029264

I only need to investigate what changed in the interaction of windows and python 3.10+ and update the type annotations to pass the type checker.

After that we should be ready for release 😃

Cheers.

[SekouDiaoNlp]

Lol I fat fingered the « close with comments » button while writing my previous message 😭

[Pebaz]

Hey @Pebaz , I fixed and updated the workflow files to the latest supported versions of Python, Nim and the github actions.

Now all tests work on all platforms of the matrix except 1 specific test, test_bdist_wheel_all_targets_installs_correctly which fails on windows + python 3.10 and 3.11.

This is the error message :

C:\hostedtoolcache\windows\Python\3.10.9\x64\lib\site-packages\coverage\report.py:114:
CoverageWarning: Couldn't parse 'C:\Users\runneradmin\AppData\Local\Temp\pip-req-build-fn4yhtcb\py_module.py':
No source for code: 'C:\Users\runneradmin\AppData\Local\Temp\pip-req-build-fn4yhtcb\py_module.py'. (couldnt-parse)

https://github.com/Pebaz/nimporter/actions/runs/3961961676/jobs/6788029105

https://github.com/Pebaz/nimporter/actions/runs/3961961676/jobs/6788029264

I only need to investigate what changed in the interaction of windows and python 3.10+ and update the type annotations to pass the type checker.

After that we should be ready for release 😃

Cheers.

Whoo hoo! Great work 😎

[SekouDiaoNlp]

Hey @Pebaz, I fully updated the type annotations and fixed all the type checker error. I re-enabled the workflow failure on type checking error and the type checking passes.

Only thing left on the CI front is the windows/python 3.10+ error on the test_bdist_wheel_all_targets_installs_correctly. I will start looking into it later today.

Cheers.

[SekouDiaoNlp]

Hey @Pebaz , do you happen to have access to a windows box to troubleshoot the windows issue? If you don’t I can whip up a windows instance on aws by next week.

Cheers.

[SekouDiaoNlp]

Meanwhile I am writing tests for the cli and integrating rich and more messages to the cli to make it more intuitive.

[Pebaz]

Hey @Pebaz , do you happen to have access to a windows box to troubleshoot the windows issue? If you don’t I can whip up a windows instance on aws by next week.

Cheers.

I’ll try to look at it this weekend and update here if I don’t get around to it. I bet you it’s some pip or distutils related update or something.

[SekouDiaoNlp]

Hey @Pebaz , do you happen to have access to a windows box to troubleshoot the windows issue? If you don’t I can whip up a windows instance on aws by next week. Cheers.

I’ll try to look at it this weekend and update here if I don’t get around to it. I bet you it’s some pip or distutils related update or something.

Hi @Pebaz , I hope you had a great weekend.

Just wanted to know if you had time to look into the windows issue.

If you didn’t have time I will setup a windows instance on aws.

Have a good day,

Peace ☮️

[Pebaz]

Hi @SekouDiaoNlp sorry I didn’t end up having any time to troubleshoot this.

[SekouDiaoNlp]

Hi @SekouDiaoNlp sorry I didn’t end up having any time to troubleshoot this.

No worries mate, weekends are for having fun 😊😊😊.

I'll spin a windows instance during the week and investigate further.

Cheers

[SekouDiaoNlp]

Hey @Pebaz , I made the changes you requested for #85 and now the stock install doesn’t install cookiecutter. At first I called the extra-option ‘dev’ but it caused conflicts with pip. After renaming it to cli, everything works fine.

The extra dependency can be installed with pip install nimporter[cli]

I will update the documentation now to reflect the changes.

Only the windows issue is left to investigate.

[SekouDiaoNlp]

Oh and I am not on my usual devbox and don’t have acces to my gpg key at the moment that’s why my commits appear as « Unverified »

[SekouDiaoNlp]

I just updated the README to inform of the optional deps

[Pebaz]

This work was incrementally added to master.