Fix error when converting from CSV

[trevorb1]

Hi!

In this PR I address the issue presented in #138.

Logic

At the top of the ReadCSV.read(...) function, I change the logic from looping over all values in the config file to only looping over values that are tagged as param or set in the config file. I have added in tests for the ReadCSV.read(...) function as well.

Questions

I was struggling a bit with how to add tests for the ReadCSV.read(...) function. The issue is that ReadCSV.read(...) takes in a directory path that holds all the csvs. While I could extract out the location of the data/ folder in the zipped Simplicity fixture, that seemed like overkill because I would be testing all the *.csv files.

I instead created a simple config file in the TestReadCSV class, and included only the relevant csvs in a new fixture directory called tests/fixtures/data. The parameters and sets included in this directory are the minimum needed to test the functionality of the ReadCSV.read(...) function (I think!)

If this was totally wrong, so sorry! Please just let me know and maybe we can discuss how to correctly test this function :)

Thanks!

[trevorb1]

Thanks for the feedback, @willu47!

I have updated this PR with the following:

• Implementing your suggestion of creating a separate function out of part of the ReadCSV.read(...) method. The new function is called _get_input_data(...) and is a copy/paste of the try except block that read in the csv data. Breaking out this function worked great because it allowed us to skip my old solution of individually checking if each value in the config file was a param or set!
• In the ReadCSV.read(...) method, we originally had a variable called config_details which was just the configuration file info for a specific value (ie. user_config[parameter]). I replaced this with the value details, which already exists at the start of the loop definition and contained the same info (ie. for parameter, details in self.user_config.items():). In the original code, details was simply an unused variable.
• Implementing your suggestion of being more strategic with the testing. I reduced the number redundant tests and instead focused on just testing the new _get_input_data(...) method. I wrote one test for each case (one for the try condition and one for the except condition). However, I think it's getting close to what you were saying before about there not being a need to test pd.read_csv(...). If you think the two tests I added arn't that helpful, Im fine to remove them :)

Thanks!

[trevorb1]

I am merging this with a couple extra additions not related to the actual issue:

1. Seems that there was a recent addition to the isort package that broke with pre-commit. Following this thread on StackOverflow, I updated the isort version in .pre-commit-config.yaml
2. This change is incompatible with Python 3.7, so I have removed checks for Python 3.7 in GitHub Actions and replaced it with Python version 3.11.
3. This isort version change is causing issues with tests with the frictionless library. I can't figure out the reason why, but since these tests will be removed with PR #141, I have just commented them out for the time being. The errors that frictionless raise are stuff like:
   • TypeError: init() got an unexpected keyword argument 'onerror'
   • frictionless.exception.FrictionlessException: [error] schema is not set or inferred

But when I run the same commands outside of pytest, everything works fine. Anyways, since these tests are going to be removed soon, I am just going to move forward.