Change local test way, directly import not install

[Zeroto521]

Brief Description

When I did PR #884, an upstream PR (#861) was merged. So I need to pull upstream to local. Then I did the local test, just typed make test. An error came out, there showed missing conditional_join. It was weird. After I checked the error stacks I got the problem. Local tests used pypi the newest version (0.21.0 ) codes, not my local codes.

So the quick fix this is by typing python setup.py develop. But why not directly use import pyjanitor in the tests folder? Just add __init__.py in each tests folder is fine.

• This way would be much easier. python setup.py develop is more like do tests in different folders (eg. another folder script needs to use the developing status code, it has to install the package into the enviroment).

• Don't worry about source codes version conflict (I came across it above). If the virtual environment has pyjanitor, pytest wouldn't import that environment's pyjanitor.

• And such those following lines could be deleted.

  https://github.com/pyjanitor-devs/pyjanitor/blob/ea8e006208f99cb567f9c6203bb260a9b88cad42/Makefile#L47-L48 https://github.com/pyjanitor-devs/pyjanitor/blob/ea8e006208f99cb567f9c6203bb260a9b88cad42/.github/workflows/tests.yml#L35

[ericmjl]

Hi @Zeroto521, thanks for chiming in. I think this could be a good idea. Does doing what you've proposed guarantee that, even if pyjanitor is installed from PyPI or conda-forge inside an environment, when we run tests from inside the cloned repository we will never hit the source code installed from PyPI/conda-forge?

On thinking further about the setup, I'm wondering if there's perhaps an advantage to setting up a completely isolated virtual environment for pyjanitor development, rather than relying on an existing environment? After all, if we standardize on the development environment we help new contributors avoid accidental dependence on external libraries that were not intended to be dependencies for pyjanitor.

[Zeroto521]

Does doing what you've proposed guarantee that, even if pyjanitor is installed from PyPI or conda-forge inside an environment, when we run tests from inside the cloned repository we will never hit the source code installed from PyPI/conda-forge?

It involves the import priority problem. import priority: script local folder => build-in lib => 3rd lib

A simple demo could show that.

.
├── whatever.py
└── random.py

# whatever.py
import random

print(random.random())

# random.py
def random():
    return

$ python whatever.py
None

Back to pyjanitor' unit test. We want to tests folder scripts import janitor folder scripts. We need to add __init__.py into tests folder firstly, then pytest would treat them as a package.

pyjanitor
├── janitor
└── tests

[ericmjl]

Got it. I learned something new today, thanks @Zeroto521. It definitely will make the initial setup magical, though the pattern is foreign to me as I've never done things this way before.

Let me ping on Twitter in my usual circles to see if this is a canonical pattern or not. Now that I know it's not technically impossible to leverage priority order to run tests, I'd like to see what other seasoned pytest users' priors are on this matter.

[hectormz]

I think the priority should be to run tests against the pyjanitor that is installed, and not what is just in the folder, since we want to test behavior when it is actually installed for a user.

I think I had gathered some inspiration from this before but I may be thinking of something else.

So for development purposes, I think it's best to always install the package in editable/developer mode (in a virtual environment/container) and not from PyPi/Conda-forge.

[hectormz]

As an aside, as mentioned in the post and in response to @ericmjl tweet about this, I have switched to the src/ layout in any new packages I create myself

[ericmjl]

@Zeroto521 I'm letting the responses collect on Twitter. Thus far, the responses from those whom I've interacted with have been in favour of our current setup, i.e. doing an editable install, rather than including __init__.py in the tests directory. On balance I'm in favour of closing the issue and marking it as a wontfix, but wanted to let you know that I appreciate you bringing it up as a suggestion - the intent is to make development easier, and I like the direction.

For now, though, I'll keep the issue open - we'll give the feedback a bit more time to trickle in.