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

Namespaces #17

Open
wants to merge 32 commits into
base: main
Choose a base branch
from
Open

Conversation

lars-reimann
Copy link
Member

@lars-reimann lars-reimann commented Apr 25, 2023

Summary of Changes

Implement namespaces as described in #14.

lars-reimann and others added 27 commits March 9, 2023 10:45
Co-authored-by: Hans-Martin von Gaudecker <[email protected]>
Co-authored-by: Hans-Martin von Gaudecker <[email protected]>
Co-authored-by: Hans-Martin von Gaudecker <[email protected]>
@codecov
Copy link

codecov bot commented Apr 25, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (645e7d5) 96.46% compared to head (43173e8) 97.71%.
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #17      +/-   ##
==========================================
+ Coverage   96.46%   97.71%   +1.24%     
==========================================
  Files           8       10       +2     
  Lines         453      699     +246     
==========================================
+ Hits          437      683     +246     
  Misses         16       16              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@MImmesberger
Copy link
Member

I found a small bug in _get_namespace_and_simple_name.

Suppose I have a tree with two leafs: a function called a and a function called _a. Then, because of the rsplit method, the package things both functions have the same name, which results in an error in an input consistency check (we have the case in GETTSIM with lohnst_m and _lohnst_m). Essentially, the implicit assumption that, in the case of triple underscores in the qualified name, the module has a trailing underscore is wrong in my case.

Now, I guess the preferred solution is to get rid of the implicit assumption all together. To do this, we probably have to rewrite a few functions. I added a failing test such that we don't forget about this.

In the mean time, I reversed the implicit assumption via 811f211 such that it suits the application.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants