MAINT: Distinguish between private and public namespaces #601

Smit-create · 2022-02-11T06:13:23Z

Related to #350

I am trying to work on #350 by using private and public namespace changes. Since ecdf.py is not meant for public namespace i.e we want users to use from quantecon import ECDF instead of from quantecon.ecdf import ECDF, we can do the following to resolve this:

Rename ecdf.py to _ecdf.py using git mv (This preserves the commit history of ecdf.py and transfers it to _ecdf.py)
Create a new file ecdf.py and add warnings (This is required at the current stage to avoid breaking the code and can be removed safely in future versions)

The current PR works in the following way:

>>> from quantecon.ecdf import ECDF  # works but throws a warning
<stdin>:1: DeprecationWarning: Please use `ECDF` from the `quantecon` namespace, the `quantecon.ecdf` namespace is deprecated.
>>> from quantecon import ECDF # works fine

We can remove the ecdf.py file in future versions and then we will just have _ecdf.py as a private namespace

cc @mmcky @jstac

pep8speaks · 2022-02-11T06:13:26Z

Hello @Smit-create! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

In the file quantecon/__init__.py:

Line 31:1: W191 indentation contains tabs
Line 31:7: E101 indentation contains mixed spaces and tabs
Line 31:8: E128 continuation line under-indented for visual indent
Line 33:1: W191 indentation contains tabs
Line 33:7: E101 indentation contains mixed spaces and tabs
Line 33:9: E128 continuation line under-indented for visual indent
Line 53:1: E265 block comment should start with '# '
Line 55:1: W191 indentation contains tabs
Line 55:4: E101 indentation contains mixed spaces and tabs
Line 55:7: E128 continuation line under-indented for visual indent
Line 56:1: W191 indentation contains tabs
Line 56:4: E101 indentation contains mixed spaces and tabs
Line 56:7: E128 continuation line under-indented for visual indent
Line 57:1: W191 indentation contains tabs
Line 57:4: E101 indentation contains mixed spaces and tabs
Line 57:7: E128 continuation line under-indented for visual indent

In the file quantecon/_compute_fp.py:

Line 114:9: E731 do not assign a lambda expression, use a def
Line 117:14: E127 continuation line over-indented for visual indent

In the file quantecon/_dle.py:

Line 11:1: E302 expected 2 blank lines, found 1
Line 13:80: E501 line too long (96 > 79 characters)
Line 14:80: E501 line too long (96 > 79 characters)
Line 16:80: E501 line too long (92 > 79 characters)
Line 38:80: E501 line too long (82 > 79 characters)
Line 41:80: E501 line too long (84 > 79 characters)
Line 44:80: E501 line too long (82 > 79 characters)
Line 45:80: E501 line too long (86 > 79 characters)
Line 51:80: E501 line too long (80 > 79 characters)
Line 53:80: E501 line too long (85 > 79 characters)
Line 63:80: E501 line too long (90 > 79 characters)
Line 65:80: E501 line too long (90 > 79 characters)
Line 66:80: E501 line too long (81 > 79 characters)
Line 68:80: E501 line too long (85 > 79 characters)
Line 85:80: E501 line too long (87 > 79 characters)
Line 92:80: E501 line too long (87 > 79 characters)
Line 97:80: E501 line too long (87 > 79 characters)
Line 103:80: E501 line too long (100 > 79 characters)
Line 104:80: E501 line too long (96 > 79 characters)
Line 106:80: E501 line too long (87 > 79 characters)
Line 123:80: E501 line too long (98 > 79 characters)
Line 148:80: E501 line too long (91 > 79 characters)
Line 150:80: E501 line too long (80 > 79 characters)
Line 151:80: E501 line too long (89 > 79 characters)
Line 154:80: E501 line too long (89 > 79 characters)
Line 215:80: E501 line too long (89 > 79 characters)
Line 218:80: E501 line too long (97 > 79 characters)
Line 220:80: E501 line too long (97 > 79 characters)
Line 233:40: E712 comparison to True should be 'if cond is True:' or 'if cond:'
Line 247:80: E501 line too long (105 > 79 characters)
Line 301:80: E501 line too long (87 > 79 characters)
Line 302:80: E501 line too long (96 > 79 characters)
Line 324:80: E501 line too long (102 > 79 characters)

In the file quantecon/_filter.py:

Line 8:1: E302 expected 2 blank lines, found 1
Line 11:1: W293 blank line contains whitespace
Line 13:1: W293 blank line contains whitespace
Line 22:73: W291 trailing whitespace
Line 25:1: W293 blank line contains whitespace
Line 41:23: E261 at least two spaces before inline comment
Line 55:25: E261 at least two spaces before inline comment
Line 58:1: W293 blank line contains whitespace
Line 62:1: W391 blank line at end of file

In the file quantecon/_graph_tools.py:

Line 156:80: E501 line too long (80 > 79 characters)

In the file quantecon/_gridtools.py:

Line 36:5: E741 ambiguous variable name 'l'
Line 119:17: E741 ambiguous variable name 'l'
Line 201:5: E101 indentation contains mixed spaces and tabs
Line 201:5: W191 indentation contains tabs

In the file quantecon/_inequality.py:

Line 153:1: W391 blank line at end of file

In the file quantecon/_kalman.py:

Line 161:18: E741 ambiguous variable name 'l'
Line 245:38: W605 invalid escape sequence '\S'
Line 260:42: W605 invalid escape sequence '\S'

In the file quantecon/_lqcontrol.py:

Line 116:9: E731 do not assign a lambda expression, use a def
Line 149:21: E128 continuation line under-indented for visual indent
Line 250:80: E501 line too long (81 > 79 characters)

In the file quantecon/_lqnash.py:

Line 128:28: E128 continuation line under-indented for visual indent
Line 130:49: E128 continuation line under-indented for visual indent
Line 140:14: E127 continuation line over-indented for visual indent
Line 142:14: E127 continuation line over-indented for visual indent

In the file quantecon/_lss.py:

Line 126:18: E741 ambiguous variable name 'l'
Line 129:18: E741 ambiguous variable name 'l'
Line 369:9: E741 ambiguous variable name 'I'

In the file quantecon/_matrix_eqn.py:

Line 160:5: E741 ambiguous variable name 'I'
Line 302:29: E127 continuation line over-indented for visual indent
Line 303:29: E127 continuation line over-indented for visual indent

In the file quantecon/_robustlq.py:

Line 109:9: E741 ambiguous variable name 'I'
Line 190:9: E741 ambiguous variable name 'I'
Line 203:80: E501 line too long (80 > 79 characters)
Line 255:9: E741 ambiguous variable name 'I'
Line 389:9: E741 ambiguous variable name 'I'

In the file quantecon/arma.py:

Line 19:80: E501 line too long (80 > 79 characters)

In the file quantecon/estspec.py:

Line 24:80: E501 line too long (80 > 79 characters)

In the file quantecon/graph_tools.py:

Line 24:80: E501 line too long (80 > 79 characters)

Comment last updated at 2022-07-11 12:58:51 UTC

mmcky · 2022-02-14T00:11:49Z

Thanks @Smit-create this is good practice and a good idea.

coveralls · 2022-02-14T00:27:13Z

Coverage decreased (-1.3%) to 94.051% when pulling a88af65 on Smit-create:issue_350_1 into 0e21854 on QuantEcon:master.

jstac · 2022-02-23T21:34:45Z

Many thanks for working on this @Smit-create . I'll leave the review to you @mmcky . Thanks in advance.

mmcky · 2022-03-23T01:37:12Z

thanks @Smit-create once we resolve the conflict I am happy for this to be merged. Thanks for putting it together.

I think this is a good idea.

Does this apply to any other modules?

Smit-create · 2022-03-23T03:39:11Z

Does this apply to any other modules?

Yeah, this applies to every private module that should not be public.

mmcky · 2022-03-23T03:56:18Z

Does this apply to any other modules?

Yeah, this applies to every private module that should not be public.

Perhaps we should update them all in the branch then and rename it to resolve #350

mmcky · 2022-03-24T20:51:16Z

@Smit-create looks like the one failure on os-x is due to a small tolerance issue. I will re-run the workflow to check if it is a stochastic error (as the exact same test didn't fail on conda-build.

Is there any reason we can't migrate to just conda testing now?

Smit-create · 2022-03-27T07:06:57Z

Is there any reason we can't migrate to just conda testing now?

Yeah, we can move to conda testing. But I'll suggest first moving all the tests to pytest and then moving to conda build.

Smit-create · 2022-03-27T07:08:02Z

if it is a stochastic error (as the exact same test didn't fail on conda-build.

It might be some platform+conda specific tolerance issue. Maybe we can slightly raise the tolerance value to make it pass.

mmcky · 2022-05-09T03:52:58Z

thanks for working on this @Smit-create. The diff makes it a bit tricky to track the changes -- for some reason the files aren't showing as modified (post a rename) but deleted and new files in place. Is this due to a force push? We will have to lean on the tests in that case. Can you ping me once the test suite is passing. Thanks.

Smit-create · 2022-05-10T05:18:31Z

The diff makes it a bit tricky to track the changes -- for some reason the files aren't showing as modified (post a rename) but deleted and new files in place

New files will have an underscore before their names, while the other files will be shown as modified.

We will have to lean on the tests in that case.

Yeah, that's true. The diff is looking lengthy to review, but it all just follows the pattern which is followed in OP.

mmcky · 2022-05-10T05:38:51Z

New files will have an underscore before their names, while the other files will be shown as modified.

Hmm. On other PR;s I have seen renamed files (with diffs). I'll have to try and find one, maybe it's because I tend to use git mv etc. but not worries. We can rely on tests when they are passing.

Smit-create · 2022-05-10T05:50:11Z

I tend to use git mv

I also have used the same. But the reason is, after renaming, I am creating a new file of the same name say:

I rename ecdf to _ecdf
I create a new file with the name ecdf and add depreciation warnings here to avoid breaking the existing code for our current users.

mmcky · 2022-05-10T06:41:22Z

I create a new file with the name ecdf and add depreciation warnings here to avoid breaking the existing code for our current users.

Ah ha -- that make sense! Thanks @Smit-create

mmcky · 2022-05-11T04:25:45Z

@jstac I will review this PR against our lecture sites. But just wanted to tag you re: updates to interface in the quantecon package. @Smit-create has done a great job at adding deprecation notices for the import changes -- so no current code out there that uses quantecon should break with this update.

The effect of this will be to make .<tab> in jupyter notebooks MUCH cleaner.

mmcky · 2022-11-22T03:37:14Z

@Smit-create sorry it has taken me so long to loop around to this. Do you mind updating to the latest master and then I'll merge?

mmcky · 2022-12-17T03:49:26Z

These changes applied, how will the documentation look?

@oyamad all public facing (importable) classes and functions should still all show up in the documentation. There may be some helper functions that won't be visible. Once this is updated I will take a closer look and see if we can do a test build of rtd

Smit-create · 2022-12-17T04:06:43Z

thanks @Smit-create I think I have all other v0.6.0 changes merged in if you have time to loop through this again to rebase on latest master

I have rebased this branch on the lastest master branch. Thanks @mmcky

oyamad · 2022-12-17T04:21:04Z

How will the development procedure go given this?
Suppose that a contributer wanted to implement something in a new file. Then they are supposed to write the code in _xxx.py and also create xxx.py in which they write from . import _xxx? (An instruction written somewhere would be helpful!)

oyamad · 2022-12-17T04:26:12Z

@Smit-create This is not something caused by this PR, but would you fix these warnings (showing up during a documentation build) (change Note to Notes)?

/Applications/anaconda3/lib/python3.10/site-packages/numpydoc/docscrape.py:460: UserWarning: Unknown section Note in the docstring of rouwenhorst in /Applications/anaconda3/lib/python3.10/site-packages/quantecon/markov/approximation.py.
  warn(msg)
/Applications/anaconda3/lib/python3.10/site-packages/numpydoc/docscrape.py:460: UserWarning: Unknown section Note in the docstring of tauchen in /Applications/anaconda3/lib/python3.10/site-packages/quantecon/markov/approximation.py.
  warn(msg)

Smit-create · 2022-12-17T04:29:07Z

Suppose that a contributer wanted to implement something in a new file. Then they are supposed to write the code in _xxx.py and also create xxx.py in which they write from . import _xxx? (An instruction written somewhere would be helpful!)

Suppose someone needs to write a new routine ABC which needs to be imported from quantecon directly, then he should just create a new file say _xxx.py, and in quantecon.__init__ import it as from ._xxx import ABC.

The current PR creates a new file xxx.py and adds some warnings in order to support backward compatibility. We can safely remove the files with xxx.py after maybe a year or two. This way we have a clear picture of what needs to be in quantecon's namespace and what needs to be in quantecon.some_sub_module. You can have a look at this #601 (comment) for a more clear explanation.

Smit-create · 2022-12-17T04:33:40Z

but would you fix these warnings (showing up a documentation build) (change Note to Notes)?

Thanks, fixed that.

Docs build log

% make html   
python qe_apidoc.py
sphinx-build -b html -d build/doctrees   source build/html
Running Sphinx v5.3.0
OMP: Info #273: omp_set_nested routine deprecated, please use omp_set_max_active_levels instead.
making output directory... done
WARNING: html_static_path entry '_static' does not exist
WARNING: Support for emitting HTML 4 output is deprecated and will be removed in Sphinx 7. ("html4_writer=True detected in configuration options)
loading intersphinx inventory from http://docs.python.org/objects.inv...
intersphinx inventory has moved: http://docs.python.org/objects.inv -> https://docs.python.org/3/objects.inv
[autosummary] generating autosummary for: game_theory.rst, game_theory/brd.rst, game_theory/fictplay.rst, game_theory/game_generators/bimatrix_generators.rst, game_theory/lemke_howson.rst, game_theory/localint.rst, game_theory/logitdyn.rst, game_theory/mclennan_tourky.rst, game_theory/normal_form_game.rst, game_theory/pure_nash.rst, ..., tools/rank_nullspace.rst, tools/robustlq.rst, util.rst, util/array.rst, util/combinatorics.rst, util/common_messages.rst, util/notebooks.rst, util/numba.rst, util/random.rst, util/timing.rst
building [mo]: targets for 0 po files that are out of date
building [html]: targets for 65 source files that are out of date
updating environment: [new config] 65 added, 0 changed, 0 removed
reading sources... [100%] util/timing                                          
looking for now-outdated files... none found
pickling environment... done
checking consistency... done
preparing documents... done
writing output... [100%] util/timing                                           
generating indices... genindex py-modindex done
highlighting module code... [ 43%] quantecon.game_theory.game_generators.bimatrihighlighting module code... [100%] quantecon.util.timing                       
writing additional pages... search done
copying static files... done
copying extra files... done
dumping search index in English (code: en)... done
dumping object inventory... done
build succeeded, 2 warnings.

The HTML pages are in build/html.

Build finished. The HTML pages are in build/html.

mmcky · 2022-12-17T04:36:52Z

It looks like rtd will not build a version for non-local branches.

@oyamad I built this locally with the results
html.zip

The top level listing looks pretty similar with no major changes.

mmcky · 2022-12-17T04:38:57Z

quantecon/__init__.py

+# Imports that should be deprecated with markov package
+from .markov import mc_compute_stationary, mc_sample_path
+
+# Imports that are deprecated and will be removed in further versions


@Smit-create do we have any infrastructure that issues a Warning for users that do import these that advises them to move towards the new import path (in version 0.7.0) etc.?

I don't want to hold up the release but if we don't then perhaps we can add this to help with migration given these are still available at the top level.

This works in the following way:

>>> from quantecon.ecdf import ECDF # works but throws a warning <stdin>:1: DeprecationWarning: Please use `ECDF` from the `quantecon` namespace, the `quantecon.ecdf` namespace is deprecated. You can use the following instead: `from quantecon import ECDF`. >>> from quantecon import ECDF

thanks @Smit-create I thought that was the case. Once we identify a deprecation point I might update these in a future release to let users know when that will happen.

mmcky · 2022-12-17T04:42:15Z

How will the development procedure go given this?
Suppose that a contributer wanted to implement something in a new file. Then they are supposed to write the code in _xxx.py and also create xxx.py in which they write from . import _xxx? (An instruction written somewhere would be helpful!)

Just saw this @oyamad -- looping round re: contribution / style guide is a great idea. I'll open an issue to track this. #665

mmcky · 2022-12-17T04:47:17Z

@oyamad are you happy for me to merge this to master. I will then loop through our lectures

python-programming
python
python-advanced

to check that the main branch is working across our projects before cutting a release (likely tomorrow or Monday).

It's a high volume change -- so I want to make sure you're OK with this :-).

mmcky · 2022-12-17T04:51:20Z

thanks @Smit-create for all your work on this. It's been a long time coming but I will merge this now and do some final testing of the release candidate on master

…ntEcon#601)" This reverts commit b0ac7e8.

Revert "MAINT: Distinguish between private and public namespaces (#601)"

Smit-create mentioned this pull request Feb 23, 2022

repeated names in directory #350

Open

Smit-create force-pushed the issue_350_1 branch from f8522ca to 24b9c55 Compare March 23, 2022 03:42

mmcky changed the title ~~MAINT: [Demo] Distinguish between private and public namespaces~~ MAINT: Distinguish between private and public namespaces Mar 23, 2022

Smit-create added enhancement maintenance labels Mar 24, 2022

Smit-create force-pushed the issue_350_1 branch from 579763e to dd4d706 Compare May 5, 2022 12:32

Smit-create force-pushed the issue_350_1 branch 2 times, most recently from 87ff9d4 to 0961efd Compare May 10, 2022 05:47

mmcky self-requested a review May 11, 2022 04:30

Smit-create force-pushed the issue_350_1 branch from 0961efd to 3994b6f Compare July 11, 2022 12:58

mmcky added this to the v0.6.0 milestone Nov 22, 2022

Smit-create force-pushed the issue_350_1 branch 2 times, most recently from e94f975 to 0ab7c73 Compare November 24, 2022 06:20

Smit-create added 4 commits December 17, 2022 09:18

MAINT: Rename files

8ada5d2

MAINT: Fix imports

22001f4

Fix merge issues

3871292

MAINT: Correct version to 0.8.0

cd0403b

Smit-create force-pushed the issue_350_1 branch from 0ab7c73 to cd0403b Compare December 17, 2022 03:52

Fix docs warning

a88af65

Smit-create added the ready label Dec 17, 2022

mmcky reviewed Dec 17, 2022

View reviewed changes

mmcky mentioned this pull request Dec 17, 2022

Update Developer Contribution Guide #665

Open

oyamad approved these changes Dec 17, 2022

View reviewed changes

mmcky self-requested a review December 17, 2022 04:50

mmcky approved these changes Dec 17, 2022

View reviewed changes

mmcky merged commit b0ac7e8 into QuantEcon:master Dec 17, 2022

Smit-create deleted the issue_350_1 branch December 17, 2022 04:53

mmcky mentioned this pull request Dec 18, 2022

Migrate Commit History? #671

Closed

Smit-create added a commit to Smit-create/QuantEcon.py that referenced this pull request Dec 20, 2022

Revert "MAINT: Distinguish between private and public namespaces (Qua…

ee40f53

…ntEcon#601)" This reverts commit b0ac7e8.

Smit-create added a commit to Smit-create/QuantEcon.py that referenced this pull request Dec 22, 2022

Revert "MAINT: Distinguish between private and public namespaces (Qua…

26aa53a

…ntEcon#601)" This reverts commit b0ac7e8.

Smit-create added a commit to Smit-create/QuantEcon.py that referenced this pull request Dec 22, 2022

Revert "MAINT: Distinguish between private and public namespaces (Qua…

45c7c04

…ntEcon#601)" This reverts commit b0ac7e8.

Smit-create added a commit that referenced this pull request Dec 31, 2022

Merge pull request #675 from Smit-create/revert-601

9ffb7d2

Revert "MAINT: Distinguish between private and public namespaces (#601)"

Smit-create mentioned this pull request Dec 31, 2022

Fix APIs and preserve commit history #680

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MAINT: Distinguish between private and public namespaces #601

MAINT: Distinguish between private and public namespaces #601

Smit-create commented Feb 11, 2022 •

edited

Loading

pep8speaks commented Feb 11, 2022 •

edited

Loading

mmcky commented Feb 14, 2022

coveralls commented Feb 14, 2022 •

edited

Loading

jstac commented Feb 23, 2022

mmcky commented Mar 23, 2022

Smit-create commented Mar 23, 2022

mmcky commented Mar 23, 2022

mmcky commented Mar 24, 2022

Smit-create commented Mar 27, 2022

Smit-create commented Mar 27, 2022

mmcky commented May 9, 2022

Smit-create commented May 10, 2022

mmcky commented May 10, 2022

Smit-create commented May 10, 2022

mmcky commented May 10, 2022

mmcky commented May 11, 2022

mmcky commented Nov 22, 2022

mmcky commented Dec 17, 2022

Smit-create commented Dec 17, 2022

oyamad commented Dec 17, 2022

oyamad commented Dec 17, 2022 •

edited

Loading

Smit-create commented Dec 17, 2022

Smit-create commented Dec 17, 2022

mmcky commented Dec 17, 2022

mmcky Dec 17, 2022

mmcky Dec 17, 2022

Smit-create Dec 17, 2022

mmcky Dec 17, 2022

mmcky commented Dec 17, 2022 •

edited

Loading

mmcky commented Dec 17, 2022 •

edited

Loading

mmcky commented Dec 17, 2022

MAINT: Distinguish between private and public namespaces #601

MAINT: Distinguish between private and public namespaces #601

Conversation

Smit-create commented Feb 11, 2022 • edited Loading

pep8speaks commented Feb 11, 2022 • edited Loading

Comment last updated at 2022-07-11 12:58:51 UTC

mmcky commented Feb 14, 2022

coveralls commented Feb 14, 2022 • edited Loading

jstac commented Feb 23, 2022

mmcky commented Mar 23, 2022

Smit-create commented Mar 23, 2022

mmcky commented Mar 23, 2022

mmcky commented Mar 24, 2022

Smit-create commented Mar 27, 2022

Smit-create commented Mar 27, 2022

mmcky commented May 9, 2022

Smit-create commented May 10, 2022

mmcky commented May 10, 2022

Smit-create commented May 10, 2022

mmcky commented May 10, 2022

mmcky commented May 11, 2022

mmcky commented Nov 22, 2022

mmcky commented Dec 17, 2022

Smit-create commented Dec 17, 2022

oyamad commented Dec 17, 2022

oyamad commented Dec 17, 2022 • edited Loading

Smit-create commented Dec 17, 2022

Smit-create commented Dec 17, 2022

mmcky commented Dec 17, 2022

mmcky Dec 17, 2022

Choose a reason for hiding this comment

mmcky Dec 17, 2022

Choose a reason for hiding this comment

Smit-create Dec 17, 2022

Choose a reason for hiding this comment

mmcky Dec 17, 2022

Choose a reason for hiding this comment

mmcky commented Dec 17, 2022 • edited Loading

mmcky commented Dec 17, 2022 • edited Loading

mmcky commented Dec 17, 2022

Smit-create commented Feb 11, 2022 •

edited

Loading

pep8speaks commented Feb 11, 2022 •

edited

Loading

coveralls commented Feb 14, 2022 •

edited

Loading

oyamad commented Dec 17, 2022 •

edited

Loading

mmcky commented Dec 17, 2022 •

edited

Loading

mmcky commented Dec 17, 2022 •

edited

Loading