Add package to FunctionReference #69
Conversation
|
@de-code , I've added a new field to the |
|
It is currently failing in the I guess the challenge for both is the same though, you want to determine the package based on the filename. I am not sure whether Python has helper function. With |
|
When tracking the references at runtime, different packages (numpy, scipy, etc.) will have their own references. R2T2 needs a why to distinguish what references correspond to each of them in order to retrieve the full reference information form the correspoinding source file. Using the path is an option but, at least for the runtime tracker, it might be unreliable (specially when there is a direct way of getting that information). For the parser, however, we might have no other option than to use the path, as you suggest. |
|
@de-code , I've updated this setting the Now, we still need to speficy a reference source so, when finding a reference in the docstrings, for example, it is able to retrieve the full reference from somewhere. The simplest way would be the via the cli that you are currently cracking, but you might have a better suggestion. |
| def test_should_parse_docstring_reference(self, temp_dir: Path): | ||
| file_path = temp_dir / "test.py" | ||
| file_path.write_text( | ||
| "\n".join(["def some_function():" ' """', " " + DOI_1, ' """']) |
There was a problem hiding this comment.
just a minor "rant" at using a code formatter. Like here it is loosing the logical formatting that is multiple lines, of course it could have used a multi-line string. It's good to be consistent though. (I just personally don't think it necessarily improves readability)
|
|
||
| if identifier in BIBLIOGRAPHY and ref in BIBLIOGRAPHY[identifier].references: | ||
| return wrapped(*args, **kwargs) | ||
|
|
||
| if identifier not in BIBLIOGRAPHY: | ||
| BIBLIOGRAPHY[identifier] = FunctionReference( | ||
| wrapped.__name__, line, source, [], [] | ||
| wrapped.__name__, line, source, package, [], [] |
There was a problem hiding this comment.
minor: with that many arguments it might be good to use keyword arguments?
| line=docstring.lineno or 0, | ||
| name=docstring.name or '', | ||
| package=docstring.package or "", |
There was a problem hiding this comment.
minor: perhaps we should allow package to be None? (see #67 (comment))
To be honest, the only reason why I added those or "" was because it wasn't marked as optional in FunctionReference and I just wanted to get passed the linter.
Yes, it doesn't seem to make sense trying to populate it if it is not going to be used in that scenario.
CLI argument sounds good to me. It's simple and explicit. |
Register the package name when adding a reference, important to later find out what source file the reference information should be look into.
Works with the runtime tracker but breaks the docstring parser 😟 . Working on it.