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

references package even if no cited functions/methods used #48

Closed
yarikoptic opened this issue Aug 22, 2015 · 7 comments
Closed

references package even if no cited functions/methods used #48

yarikoptic opened this issue Aug 22, 2015 · 7 comments
Labels

Comments

@yarikoptic
Copy link
Member

@mvdoc Why sklearn listed (not that it shouldn't since we use its functionality here but none of the decorated methods was called AFAIK)?

$> python -m duecredit examples/example_scipy.py
I: Simulating 4 blobs           
I: Done clustering 4 blobs

DueCredit Report:
- scipy (v 0.14.1) [1]
  - scipy.cluster.hierarchy:linkage (v 0.14.1) [2]
- sklearn (v 0.16.1) [3]

2 packages cited
0 modules cited
1 functions cited

References
----------

[1] Jones, E. et al., 2001. SciPy: Open source scientific tools for Python.
[2] Sibson, R., 1973. SLINK: an optimally efficient algorithm for the single-link cluster method. The Computer Journal, 16(1), pp.30–34.
[3] Pedregosa, F. et al., 2011. Scikit-learn: Machine learning in Python. The Journal of Machine Learning Research, 12, pp.2825–2830.
@mvdoc
Copy link
Member

mvdoc commented Aug 23, 2015

well -- why not? we use it, so it's referenced :-)

@yarikoptic
Copy link
Member Author

but we are not using any duecredited function... how do we know that we don't just import it?

@mvdoc
Copy link
Member

mvdoc commented Aug 24, 2015

that's fair -- I will look into it

On Sun, Aug 23, 2015 at 3:53 PM, Yaroslav Halchenko <
notifications@github.com> wrote:

but we are not using any duecredited function... how do we know that we
don't just import it?


Reply to this email directly or view it on GitHub
#48 (comment).

@mvdoc
Copy link
Member

mvdoc commented Sep 3, 2015

So I'm looking into this, AFAIU the issue comes from injector.py that is set up to always import the package if the module/functions are imported...but I don't know what would be the safest way to change it since I have to digest the code a bit more. Any suggestion? Here's the relevant debugging output

2015-09-03 16:44:09,730 [DEBUG] Processing delayed injection for sklearn (injector.py:141)
2015-09-03 16:44:09,730 [Level 3] Importing duecredit.injections.mod_sklearn (injector.py:146)
2015-09-03 16:44:09,730 [DEBUG] Performing undecorated import of entries (injector.py:237)
2015-09-03 16:44:09,730 [Level 3] Calling injector of duecredit.injections.mod_sklearn (injector.py:157)
2015-09-03 16:44:09,730 [DEBUG] Adding citation entry BibTeX('@article{pedregosa2... for sklearn:None (injector.py:123)
2015-09-03 16:44:09,730 [DEBUG] Adding citation entry Doi('10.1126/science.113680... for sklearn.cluster.affinity_propagation_:None (injector.py:123)
2015-09-03 16:44:09,731 [DEBUG] Adding citation entry Doi('10.1.1.140.301', key='... for sklearn.cluster.bicluster:SpectralCoclustering (injector.py:123)
2015-09-03 16:44:09,731 [DEBUG] Adding citation entry Doi('10.1.1.135.1608', key=... for sklearn.cluster.bicluster:SpectralBiclustering (injector.py:123)
2015-09-03 16:44:09,731 [DEBUG] Adding citation entry Doi('10.1145/233269.233324'... for sklearn.cluster.birch:Birch (injector.py:123)
2015-09-03 16:44:09,731 [DEBUG] Adding citation entry Url('https://code.google.co... for sklearn.cluster.birch:Birch (injector.py:123)
2015-09-03 16:44:09,731 [DEBUG] Adding citation entry BibTeX('@inproceedings{este... for sklearn.cluster.dbscan_:dbscan (injector.py:123)
2015-09-03 16:44:09,731 [DEBUG] Adding citation entry Doi('10.1109/34.1000236', k... for sklearn.cluster.mean_shift_:MeanShift (injector.py:123)
2015-09-03 16:44:09,731 [DEBUG] Adding citation entry Doi('10.1109/ICCV.2003.1238... for sklearn.cluster.spectral:discretize (injector.py:123)
2015-09-03 16:44:09,731 [DEBUG] Adding citation entry Doi('10.1109/34.868688', ke... for sklearn.cluster.spectral:spectral_clustering (injector.py:123)
2015-09-03 16:44:09,731 [DEBUG] Adding citation entry Doi('10.1007/s11222-007-903... for sklearn.cluster.spectral:spectral_clustering (injector.py:123)
2015-09-03 16:44:09,731 [DEBUG] Adding citation entry Doi('10.1023/A:101093340432... for sklearn.ensemble.forest:RandomForestClassifier.predict_proba (injector.py:123)
2015-09-03 16:44:09,731 [DEBUG] Adding citation entry Doi('10.1023/A:101093340432... for sklearn.ensemble.forest:RandomForestRegressor.predict (injector.py:123)
2015-09-03 16:44:09,731 [DEBUG] Adding citation entry BibTeX('@BOOK{breiman-fried... for sklearn.tree.tree:DecisionTreeClassifier.predict_proba (injector.py:123)
2015-09-03 16:44:09,731 [DEBUG] Process 1 injections for module sklearn (injector.py:175)
2015-09-03 16:44:09,731 [Level 4] Considering 1 records for decoration of None:None (injector.py:197)
2015-09-03 16:44:09,731 [Level 1] Collector added entry pedregosa2011scikit (collector.py:204)
2015-09-03 16:44:09,732 [Level 3] Citing directly None:None since obj_path is empty (injector.py:207)
2015-09-03 16:44:09,732 [Level 3] Done processing injections for module sklearn (injector.py:210)
2015-09-03 16:44:09,732 [Level 1] Returning <module 'sklearn.datasets' from '/usr/local/lib/python2.7/site-packages/sklearn/datasets/__init__.pyc'> (injector.py:276)

@mvdoc mvdoc changed the title references module even if no cited functions/methods used references package even if no cited functions/methods used Sep 3, 2015
@yarikoptic
Copy link
Member Author

the analysis logic should happen during output, as it was before somewhat -- if there are no references triggered for submodules -- do not output that module reference (unless cite_module=True for that module, as we do for numpy only atm)

@mvdoc
Copy link
Member

mvdoc commented Sep 3, 2015

Gotcha. Thanks for the hint

@yarikoptic
Copy link
Member Author

closed by #86

yarikoptic added a commit that referenced this issue Jun 17, 2016
[Full Changelog](0.5.0...0.6.0)

**Implemented enhancements:**

- Support system-specific references [\#81](#81)
- export to bibtex doesn't support tags yet [\#19](#19)
- ENH: support DUECREDIT\_REPORT\_ALL=1 to report all citations, not only with functionality used [\#92](#92) ([yarikoptic](https://github.com/yarikoptic))

**Fixed bugs:**

- Outputting to bibtex doesn't filter by used citations [\#68](#68)
- references package even if no cited functions/methods used [\#48](#48)
- When injecting multiple citations at the same point, only one referenced [\#47](#47)

**Merged pull requests:**

- BF: allow multiple injections at the same path, avoid resetting \_orig\_import if already deactivated [\#91](#91) ([yarikoptic](https://github.com/yarikoptic))
- DOC: Update readme to reflect current output of duecredit summary [\#89](#89) ([mvdoc](https://github.com/mvdoc))
- enable codecov coverage reports [\#87](#87) ([yarikoptic](https://github.com/yarikoptic))
- REF,ENH: refactor {BibTeX,Text}Output into Output class with subclasses [\#86](#86) ([mvdoc](https://github.com/mvdoc))

* tag '0.6.0': (39 commits)
  CHANGELOG
  DOC: clarify DUECREDIT_REPORT_ALL usage
  ENH: use any instead of list when filtering packages
  DOC: Update readme to show how to use the new flag
  TST: test also DUECREDIT_REPORT_TAGS while we're at it
  TST: test all the flags
  TST: test that DUECREDIT_REPORT_ALL works
  ENH: support DUECREDIT_REPORT_ALL=1
  DOC: Update readme
  DOC: fix small change to readme
  DOC: update readme to reflect current output
  ENH: do not demand coverage for the code we hope should never be triggered
  BF: exec to figure out version should be done with locals(), globals()
  ???: naively hoping that previous bug triggered on travis would be gone fixed by prev fix
  ENH+RF: track per injector either it is active and do not deactivate in __del__ if not
  TST: add test for double injection
  BF+ENH(LOG): decorate prev decorated object during injection + improve logging
  enable codecov coverage reports
  Fix Py3 annoyances
  BF: BibTeXOutput returns only unique bibtexs
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants