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

Adding test_imports. Issue 12110 #12229

Closed
wants to merge 9 commits into from

Conversation

Projects
None yet
2 participants
@JoshuaRM
Copy link
Contributor

commented Feb 7, 2019

Summary

Issue #12110 discussed adding a test for imports in the keras/keras and keras/tests directories, where imports should be relative and absolute respectively. This PR is not in a merge state yet, but I am looking for guidance/advice on further steps. Currently the tool works as a stand alone, as I have yet to figure out integration with Travis CI.

The challenge is checking only import statements relevant to the keras function and classes, so currently the tool uses an ignore list which is not very efficient. The issue seems to be dead so I am hoping this PR can bring some attention to this.

Any advice, comments, or questions are welcome

Related Issues

PR Overview

  • This PR requires new unit tests [y/n] (make sure tests are included)
  • This PR requires to update the documentation [y/n] (make sure the docs are up-to-date)
  • This PR is backwards compatible [y/n]
  • This PR changes the current API [y/n] (all API changes need to be approved by fchollet)

JoshuaRM added some commits Oct 14, 2018

Update backend.md
Added documentation for loading an external backend
Created documentation for switching to external backend
Added documentation to the "Switching from one backend to another" section of the documentation
@gabrieldemarmiesse

This comment has been minimized.

Copy link
Member

commented Feb 8, 2019

Sorry, I judt saw you comment in the issue. I belive we should work with an AST to do that. It's much more robust. Can you look into this? https://breadcrumbscollector.tech/writing-custom-checkers-for-pylint/

@gabrieldemarmiesse

This comment has been minimized.

Copy link
Member

commented Feb 8, 2019

Closing this PR as another approach would be better suited. @JoshuaRM don't hesitate to contact me in the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.