Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

Allowing convenient selection of checked error codes #96

Closed
Nurdok opened this issue Jan 20, 2015 · 8 comments
Closed

Allowing convenient selection of checked error codes #96

Nurdok opened this issue Jan 20, 2015 · 8 comments

Comments

@Nurdok
Copy link
Member

Nurdok commented Jan 20, 2015

#0 Introduction

In this issue, I'm going to list a number of related features that I feel deserve a clear solution that can't be achieved by solving each one independently. I also suggest a possible solution. *I highly encourage anyone to criticize the features and the solution. Of course everyone is also encouraged to suggest alternative solutions, use cases or related features. In short - show me where I'm wrong.
#1 Features

  1. Allow the user to pick a "set" of error codes to comply with a certain
    standard. For example, choosing between Google style, numpy style and PEP257
    (which should be the default behavior).
  2. Allow the user to manually set which error codes are ignored when pep257 is
    run (blacklist)
  3. Allow the user to manually set which error codes are considered when pep257
    is run (whitelist)
  4. Allow the user to set a docsring convention (as in (1)) and set which
    error codes are ignored or selected with respect to the chosen convention.
  5. Allow the user to specify an ignored error code list or convention in a
    configuration file and also override it completely with CLI flags.
  6. Allow the user to specify an ignored error code list or conevention in a
    configuration file and also add to it with CLI flags.
    pep257.py contains PEP-257 errors #2 Status Quo

Right now, the only implemented feature is (2):

pep257 --ignore=D203,D400,D102

#3 Proposed Solution

3.1 Flags

--select=D100,D202
--ignore=D100,D202
--add-ignore=D100,D202
--add-select=D100,D202
--convention=numpy

3.2 Logic

  1. --select, --ignore and --convention are mutually exclusive. It's
    illegal to specify more than one of them. However, it is possible to specify
    one such flag in the config file and another one (or even the same one) as
    a command line flag, in which case the flag specified in the config file
    will be completely ignored.
  2. Not specifying either one of the above flags will be treated as if
    --convention=pep257 was specified.
  3. The value of the above flags (or the value of the default convention)
    determines a list of error codes that should be checked.
  4. --add-ignore and --add-select can only be specified as command line
    flags. Instead of fully determining the list of errors that should be
    checked, they alter the list defined by the previously mentioned flags.
  5. --add-ignore and --add-select are not mutually exclusive. However,
    specifying the same error code in both is considered illegal.
  6. Some error codes are mutually exclusive. After determining the final list
    of error codes that should be checked, it is considered illegal if two or
    more mutually exclusive flags in it.
    Handle IOError when file can't be opened #4 Use Cases

For the sake of the following use cases, assume that there exists two mutually
exclusive error codes, D101 and D102. PEP257 uses D101 (so it's on by default)

  • D100 - used by all conventions.
  • D101 - used by PEP257, but not numpy. Mutually exclusive with D102.
  • D102 - used by numpy, but not PEP257. Mutually exclusive with D101.
  • D103 - used by PEP257, but not numpy.
  • D104 - used by numpy, but not PEP257.

Also assume that there are many more errors of either of these types.

4.1 As the user, I want to use the PEP257 convention

pep257

4.2 As the user, I want to use the PEP257 convention, but also check for D104

pep257 --add-select=D104

4.3 As the user, I want to use the PEP257 convention, but check D102 instead of D101

pep257 --add-ignore=D101 --add-select=D102

4.4 As the user, I don't care about specific conventions. I just want to specifically check for D100 and D102

pep257 --select=D100,D102

4.5 As the user, I want to use the numpy convention, but also check for D103

pep257 --convention=numpy --add-select=D103

4.6 As the user, I want to use the numpy convention, but also ignore D104

pep257 --convention=numpy --add-ignore=D104

4.7 As the user, I want to use the numpy convention, but check D101 instead of D102

The following flags should achieve this:

pep257 --convention=numpy --add-select=D101 --add-ignore=D102

4.8 As the user, I want to usually check for D100, D102 and D104, but sometimes ignore D104 when I run pep257 manually

Config file:

[pep257]
select = D100,D102,D104

Command line:

pep257 --add-ignore=D104

4.9 As the user, I want to usually check for D100, D102 and D104, but sometimes also check for D103 when I run pep257 manually

Config file:

[pep257]
select = D100,D102,D104

Command line:

pep257 --add-select=D103

4.10 As the user, I want to usually check for D100, D102 and D104, but sometimes I want to check by numpy standards instead.

Config file:

[pep257]
select = D100,D102,D104

Command line:

pep257 --convention=numpy
@Nurdok
Copy link
Member Author

Nurdok commented Jan 20, 2015

This issue is probably a blocker for #91.

@jayvdb
Copy link
Member

jayvdb commented Jan 20, 2015

I would like to see pep257 using the same arg/config names as pep8 , and use roughly the same order of precedence of applying those args/config. Ideally this becomes a shareable library , so new tools can provide the same configuration options, and their code & time can focus on the linting rules. (flake8 does provide some level of normalisation across these tools, but it would be better they didnt all behave so differently on the cmdline)

@lvh
Copy link

lvh commented Jan 21, 2015

This looks fine to me.

@ricordisamoa
Copy link

👍

@xZise
Copy link

xZise commented Aug 1, 2015

I know I'm a bit late for the party, but I just want to mention this as this bug has been referenced in #91 in which I'm interested.

I find the mutually exclusive error codes handling cumbersome because if one of them is selected you need to disable one and enable the other. So quickly testing with the other mode involves to mention two codes. I'm not sure if it couldn't automatically ignore colliding codes implicitly. Unfortunately there are no examples for mutually exclusive error codes (afaik) so there is probably no current solution.

Regarding #91 which mentioned this bug, it seems like the issue there is a bit different from this here. If there are two different codes, one for 1 line separation and one for 0 lines separation they are not strictly exclusive (you could allow both separations). But especially when looking at that pull request, a new system could allow “options” for each specific code. This doesn't mean that the commands suggested here does need to provide this functionality as I've suggested some alternatives.

By the way there are a few inconsistencies in the examples above, which don't really pose a problem to be bug itself but may be (or in one case are still to me) confusing:

  • Section 4: The introduction says D100 and D101 are exclusive but the list and examples use D101 and D102.
  • Section 4: The sentence “PEP257 uses D100 (so it's on by default)” sounds superfluous compared to the list below it. As D100 is the only mentioned rule it sounds like D100 is some how special but it's not clear to me how. Is D100 always enabled so even if you choose --convention numpy it's enabled unless numpy specifically disables it? This seems counter intuitive to the --convention parameter which should afaik just be like a default list of selected xor ignored codes (basically a shortcut).
  • Section 4.2: Uses --add-ignore=D104 to enable D104. I think you meant --add-select.
  • Section 4.6: Uses --add-ignore=D103 to ignores D103 which is selected in numpy by default but header mentions D104.

@Nurdok
Copy link
Member Author

Nurdok commented Aug 1, 2015

@xZise I think the main point of difference is your sentence:

If there are two different codes, one for 1 line separation and one for 0 lines separation they are not strictly exclusive (you could allow both separations).

I disagree. If one error is for not having exactly 1 line breaks and one for not having exactly 0 line breaks, they are mutually exclusive. If you want to allow both, simply ignore both error codes. This will allow you to put any number of line breaks before class docstrings. I understand that you may want an option that allows 0 or 1 line breaks, but I don't think such an option is sensible in the same way that we don't allow other "count" issues to be configured. Standards are a way to make code look similar. If you allow to choose between several ways of doing something, it isn't really a standard.

Your comments about the inaccuracies of this issues are correct and will be edited.

@xZise
Copy link

xZise commented Aug 1, 2015

Well the main reason to allow both is to allow a conversion time where the number may be 0 or 1 (but nothing else, which is not possible when we ignore both codes). In fact for Jenkins flake8 validation on pywikibot there could be two different tests. One non-voting one allow only 0 (the new standard) one one voting one allow both. Over time it'll drift towards 0 as new classes should follow the new style.

Now in the specific case of #91 there is only one additional line so in theory it would be possible to fix all at once relatively easy by just removing one newline. But other future changes to PEP257 might be more complex where it would be advantageous to support both the old and new standard.

@Nurdok
Copy link
Member Author

Nurdok commented Aug 2, 2015

I understand your position, but I believe that disabling both error codes for the transition is good enough, and a reasonable trade-off to keep a sensible and consistent error model. Your last paragraph seems to actually support this case. Other future change might be more complex in a way that treating each one of them with a specialized flag will bloat the script. They may also be more explicitly mutually exclusive, so this problem might happen again in the future.

My current strategy is this:

  1. All error codes from all convention are included in a big list.
  2. If more than one convention includes the same "rule", there will be only one error code representing it.
  3. If there is a similar "rule" for several conventions, but the "configuration" is different (such as in this case), it should be reflected in multiple error codes.
  4. Error codes may conflict.
  5. The user can choose whichever error codes he wants from the list, as long as they are not mutually exclusive.
  6. The default checked error codes are those in the most recent version of the PEP257 document.

@Nurdok Nurdok closed this as completed Aug 2, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants