Raise a TypeError if wrong type given to fix_url

[eode]

Description

This is in relation to "Package.set_dir() should be more restrictive to path".

Note that I've modified fix_url instead of set_dir(). fix_url() is the centralized place for checking local-style paths. All tests passed with this small change, but it could cause people to need to adapt, if they have code dependent on the previous behavior.

If changing this here is considered too broad, I can shift it into set_dir.

TODO

• [x] Unit tests
• [n/a] Automated tests (e.g. Preflight)
• [x] Documentation
  • [x] Python: Run build.py for new docstrings
  • [n/a] JavaScript: basic explanation and screenshot of new features
• [?] Changelog entry

[codecov[bot]]

Codecov Report

Merging #2936 (8e72bac) into master (5f2ec40) will increase coverage by 0.01%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #2936      +/-   ##
==========================================
+ Coverage   39.65%   39.67%   +0.01%     
==========================================
  Files         597      597              
  Lines       26951    26959       +8     
  Branches     3919     3919              
==========================================
+ Hits        10687    10695       +8     
  Misses      15143    15143              
  Partials     1121     1121              

Flag	Coverage Δ
api-python	90.72% <100.00%> (+0.01%)	:arrow_up:
catalog	12.53% <ø> (ø)
lambda	87.30% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
api/python/quilt3/packages.py	92.89% <ø> (ø)
api/python/quilt3/util.py	83.44% <100.00%> (+0.16%)	:arrow_up:
api/python/tests/test_util.py	100.00% <100.00%> (ø)

:mega: Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

[eode]

It could be worth looking up functions that use fix_url and updating their docstrings as well. Or, moving the change out of fix_url and into set_dir(). But, this seems like an opportunity to start migrating to universal behavior for raising errors for places where we accept an FS path.

[eode]

@sir-sigurd Ran across something.

(master) This is a weird behavior:

>>> p = Package()
>>> p.set_dir('example')
>>> p.set_dir('')
# Result: Package is set to PWD.

>>> p = Package()
>>> p.set_dir('')
>>> p.set_dir('example')
# Result: Package is set to PWD.

That seems like buggy behavior. Is it actually intentional?

Extended example:

>>> p = Package()
>>> p.set_dir('')
(local Package)
 └─bar
 └─example/
   └─baz
   └─bing
 └─foo
>>> p.set_dir('example')
(local Package)
 └─bar
 └─example/
   └─baz
   └─bing
 └─foo
>>> p = Package()
>>> p.set_dir('example')
(local Package)
 └─example/
   └─baz
   └─bing
>>> p.set_dir('')
(local Package)
 └─bar
 └─example/
   └─baz
   └─bing
 └─foo

I'd imagine that it should a: always set the package dir, or b: set the package dir once and create an error in other circumstances.

Can you specify what the behavior should be? I'm working on that chunk of code, and may as well fix this if it's a bug.

[sir-sigurd]

I don't understand what's unexpected. From docs:

Package.set_dir(self, lkey, path=None, meta=None, update_policy='incoming') ... path(string): path to scan for files to add to package. If None, lkey will be substituted in as the path.

Does this make sense?

[eode]

Nevermind, I was thinking of set_dir as setting the package contents to the specified tree, rather than adding the specified tree to the package. Still an RTFM issue, though -- my mistake.