TST: Test no-file for source#493
Conversation
| dict[int, list[str]] | ||
| Mapping of line number to a list of checks to ignore. | ||
| """ | ||
| with open(filepath) as file: |
There was a problem hiding this comment.
I would tweak this slightly.
numpydoc_ignore_comments = {}
try:
with open(filepath) as file:
last_declaration = 1
declarations = ["def", "class"]
for token in tokenize.generate_tokens(file.readline):
if token.type == tokenize.NAME and token.string in declarations:
last_declaration = token.start[0]
if token.type == tokenize.COMMENT:
match = re.match(IGNORE_COMMENT_PATTERN, token.string)
if match:
rules = match.group(1).split(",")
numpydoc_ignore_comments[last_declaration] = rules
except (OSError, TypeError): # can be None, nonexistent, or unreadable
pass
return numpydoc_ignore_commentsThere was a problem hiding this comment.
I actually prefer the current approach as I always try to limit the scope of try/except as much as possible. With your code there is a single return path, which is nice. However the scoping for the try/except is broader, which means you risk accidentally catching a TypeError somewhere in the remaining 10 lines that do more than just "try to open the file". And I don't think the interpretation of the current code is too bad: "try to open the file and if you can't, return an empty dict. if you can, proceed to parse it"
|
I also realized that I added a |
|
Thanks! |
Not sure this is the best solution but it at least allows things in MNE-Python to pass. Turns out the real issue was us subclassing
listand that having to associatedfile. But I added another test that failed a different way along the way as well for where we create functions dynamically.cc @stefmolin @stefanv since you've probably thought about this more than I have! And @jarrodmillman since we should probably get some form of this in for 1.6.
BTW the time to test docstrings in MNE-python went from this before #476:
to:
So we should indeed prioritize time/efficiency at some point soon :)