Remove f2py dependency #421
Conversation
anissa111
added
refactor
|
Please feel free to critique how I've set up the "fake" functions to act as deprecation warnings here. |
geocat/comp/deprecated.py
Outdated
| def grid_to_triple(*args, **kwargs): | ||
| r""".. deprecated:: 2023.06.0 | ||
| To use this function, install the geocat-f2py package and import as | ||
| ``geocat.f2py.grid_to_triple``. |
There was a problem hiding this comment.
Through deprecated.py the user will already have access to the func name grid_to_triple (likewise, other functions in this script). If we guide them to import -f2py functions as
import geocat.f2py.grid_to_triple
Wouldn't that lead to namespace issues as they will have two functions in the same name importefrom two different packages? (didn't test it though)
If so, how about guiding the user to import as
import geocat.f2py
Then, call functions as:
geocat.f2py.triple_to_grid
There was a problem hiding this comment.
Ooh, super good point. I'll test the behavior locally and update this comment and the deprecated warnings accordingly.
There was a problem hiding this comment.
Okay! I have an answer. This should actually be fine the way we have it now. If two packages have the same function name imported into the same namespace, the latter one will override the prior one.
This is consistent with my testing locally. If a user imports the geocat-f2py functions on top of the same namespace, they will override the fake geocat-comp ones.
There was a problem hiding this comment.
Is it possible someone could mix up the order though? That makes me a little nervous.
There was a problem hiding this comment.
Actually, I feel safer after @anissa111 's explanation and testing. Approving it.
If we wanted to cover the case @kafitzgerald pointing out though, I am curious if deprecated.py functions could be written in a way that if there is no geocat-f2py, show the deprecation message; else, call the geocat.f2py.function.
This would be an overkill though.
There was a problem hiding this comment.
@kafitzgerald If someone managed to install geocat-f2py and then geocat-comp, they would just see the message to import with geocat.f2py.<function>, which will work no matter which package was installed first
There was a problem hiding this comment.
Ah, right. Ignore my comment.
PR Summary
Closes #420
Summary of changes:
deprecated.pyto house fake f2py deprecation functions that direct users to try installing geocat-f2py directlyNote: this will also require corresponding changes in the feedstock repo when the next release is being made
PR Checklist
General
closes #XXXto the PR description where XXX is the number of the issue.docs/release-notes.rstprecommit. To set up on your local, runpre-commit installfrom the top level of the repository. To manually run pre-commits, usepre-commit run --all-filesand re-add any changed files before committing again and pushing.