Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unused parameter or use it #1332

Closed
lavigne958 opened this issue Oct 25, 2023 · 3 comments · Fixed by #1343
Closed

Remove unused parameter or use it #1332

lavigne958 opened this issue Oct 25, 2023 · 3 comments · Fixed by #1343
Labels
Need investigation This issue needs to be tested or investigated

Comments

@lavigne958
Copy link
Collaborator

Overview

I noticed the following unused parameter, we should either use it, or remove it.

use_index=0,

I'll have a look at the code itself a bit longer to check what is best.

@alifeee
Copy link
Collaborator

alifeee commented Oct 25, 2023

Hi. use_index was added by #1301 (see #1301 (comment)). Currently, it does nothing if not set to use_index=0

if use_index not in [0, 1]:
raise ValueError("use_index must be either 0 or 1")
if use_index == 1: # TODO: implement use_index=1
raise NotImplementedError("use_index=1 is not implemented yet")

It exists so that #808 can be implemented. If use_index = 1 were used, then we could get records using the first column as headers, instead of the first row.

@alifeee alifeee added the Need investigation This issue needs to be tested or investigated label Oct 25, 2023
@lavigne958
Copy link
Collaborator Author

alright so for now:

  • it has no impact
  • if a user set it to the wrong value it will raise an exception and stop the program
  • we don't plan to use it so far

so let's remove it. it's nothing big we can add it back later when we need.

I suggest.

@alifeee
Copy link
Collaborator

alifeee commented Nov 5, 2023

agreed. made a PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Need investigation This issue needs to be tested or investigated
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants