Skip to content
This repository was archived by the owner on Jan 4, 2025. It is now read-only.

Deprecation warnings for using old arguments in request methods.#222

Merged
GriceTurrble merged 12 commits intodevelopfrom
feature-deprecated-arguments
Nov 3, 2020
Merged

Deprecation warnings for using old arguments in request methods.#222
GriceTurrble merged 12 commits intodevelopfrom
feature-deprecated-arguments

Conversation

@GriceTurrble
Copy link
Copy Markdown
Member

@GriceTurrble GriceTurrble commented Oct 7, 2020

Adds a decorator, kwargs_renamed_for_v11, that allows the use of keyword arguments from the 0.8 branch for easier adoption of 1.0. If a kwarg is used with the 0.8.x naming, a DeprecationWarning is raised (specifically a RemovedInPAM11Warning), then the kwarg is mapped to the new name automatically so the operation can continue unimpeded.

Example:

# mws/apis/feeds.py

class Feeds(MWS):
    ...

    @kwargs_renamed_for_v11([("marketplaceids", "marketplace_ids")])
    def submit_feed(
        self,
        feed,
        feed_type,
        feed_options=None,
        marketplace_ids=None,
        amazon_order_id=None,
        document_type=None,
        content_type="text/xml",
        purge=False,
    ):

Here, the 0.8 version of submit_feed uses the argument marketplaceids, which has been renamed in 1.0dev versions to marketplace_ids. It may be unclear to some users that this change was made without combing through documentation or source code, and the operation would fail if the user called the method using submit_feed(..., marketplaceids="foo").

With the decorator in place, the operation will continue as before, simply raising a warning that details the name change. The value "foo" is then passed into the decorated method as marketplace_ids, the kwarg expected by the new function signature.

Testing

Test forthcoming, will update this PR once added.

Raises warning, re-maps args to new names, and continues operations.
Aids in uptake of 1.0 version for users of 0.8
@codecov
Copy link
Copy Markdown

codecov bot commented Oct 7, 2020

Codecov Report

Merging #222 into develop will increase coverage by 1.14%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #222      +/-   ##
===========================================
+ Coverage    81.45%   82.59%   +1.14%     
===========================================
  Files           30       30              
  Lines         1337     1385      +48     
  Branches       144      146       +2     
===========================================
+ Hits          1089     1144      +55     
+ Misses         220      211       -9     
- Partials        28       30       +2     
Impacted Files Coverage Δ
mws/apis/inventory.py 100.00% <ø> (ø)
mws/apis/feeds.py 63.88% <100.00%> (+10.85%) ⬆️
mws/apis/inbound_shipments.py 100.00% <100.00%> (ø)
mws/apis/orders.py 100.00% <100.00%> (ø)
mws/apis/products.py 94.87% <100.00%> (+1.12%) ⬆️
mws/apis/recommendations.py 100.00% <100.00%> (ø)
mws/apis/reports.py 89.33% <100.00%> (+2.00%) ⬆️
mws/utils/deprecation.py 100.00% <100.00%> (ø)
mws/apis/easyship.py 29.72% <0.00%> (ø)
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3bdeaee...e054df2. Read the comment docs.

Copy link
Copy Markdown
Member

@Bobspadger Bobspadger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All looks sensible to me so far - just some tests to make sure it works as anticipated.

@GriceTurrble GriceTurrble marked this pull request as draft October 21, 2020 15:39
Comment on lines 27 to +29
response_group="Basic",
next_token=None,
marketplace_id=None,
next_token=None,
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a reordering change here, should be no impact.

@samamorgan
Copy link
Copy Markdown
Contributor

Is there a lifecycle for this chance? I'd hope that this deprecation warning wouldn't stay in place in perpetuity.

Introduces api_instance fixture, which can read the api_class
from a requesting test class in order to instantiate
the correct API for a set of tests.

In implementing this with a small change in MWS base tests,
found that mws_credentials fixture including the auth token
broke a test that required the token *not* be set.
So, broke that into a second fixture that explicitly includes
the auth_token, and added a second fixture for api_instance
that also includes the auth_token, for clarity.
The main focus of this feature, of course, is raising deprecation warnings
when users attempt to use these request methods
with their older argument names (which were renamed for consistency earlier).

This change adds in tests that ensure each of these methods raises
appropriate warnings when the old argument name is called,
and that no warning is raised when the new one is used.
It further tests that the request parameters generated by each request
are identical, whether using the old name or the new name.

The real work of each test takes place in MethodRenamedBase.run_method,
while each test case supplies the precise method, kwarg names, and required arguments
in order for a minimal request to pass
(with old and new names injected with paramtrization
and required args supplies within the methods).

Note how the test will also remove a "required" arg
if the changed arg is one of them, ensuring that a
real test of that arg (when named) also occurs.
@GriceTurrble GriceTurrble marked this pull request as ready for review October 22, 2020 21:02
Initially found a test failure due to the Timestamp not matching.
This is a minor race condition, as there are two requests being generated.

However, this isn't really a part of this particular test,
where we're more interested in the Action and other attrs
specific to the request.

It's also possible the SIgnature key could be screwy,
as that's a hash of the timestamps, as well.

So, in the end, I've thrown in most param keys
that are common to all MWS requests.
Everything left after this should be relevant to this test.
@GriceTurrble
Copy link
Copy Markdown
Member Author

GriceTurrble commented Oct 23, 2020

@samamorgan These are meant to create a bridge between 0.8 and 1.0dev versions to aid in upgrading off 0.8. Users that are already on 1.0dev and using param names as defined in the method signatures should not see deprecation warnings at all (that's actually part of the tests I pushed last night!): while users coming in from version 0.8 using incorrect names will see warnings raised only if they use the "old" argument names.

This corrects an issue where users may attempt to use the "old" names in 1.0dev, which previously would just throw an error and prevent the request from going through. With this update, that same code would successfully send a request, while also warning the user to update their code for the future. Particularly for folks who are still using 0.8 from a PyPI install, this should smooth over the transition once we have a 1.0 release up on PyPI and those upgrades start rolling in.

As for lifecycle, the warning raised is RemovedInPAM11Warning, as I expect to remove these deprecated calls in v1.1. No specific timeline on that yet, though.

Tests are being updated in another set of changes, and the changes
made here will certainly create a conflict.

This adjustment normalizes those changes, making the code identical.
Test passes this way in both cases, even without changing the other
fixtures to match what's incoming.
@GriceTurrble
Copy link
Copy Markdown
Member Author

Note on latest commit today: changes I made to that test prior would make a conflict. The changes I've made now match up with incoming changes from another feature set. Should help avoid a conflict down the line, and still passes either way.

Kinda weird to do it that way, but I can see it being an issue without it.

GriceTurrble added a commit that referenced this pull request Oct 24, 2020
Now properly using pytext and fixtures to fullest extent
across the InboundShipments API class.

Fixtures for "cred_X_Y" removed, as they were really not needed.
Replaced with constants that I can access within non-test methods,
such as common_assert_params.
Other methods that might have used those fixtures
can instead use `mws_credentials` and remove options as needed.

An api_instance fixture is included, which can read request context
and take an api class from the calling class.
This requires it be used in classes that set `api_class`,
but that's a minor concern.

Common tooling for the newer tests is similar to old, with tweaks to work
with pytest fixtures more cleanly.
No more setup logic for self.api, hello api_instance fixture in
common tests!

All reliance on unittest removed from the test cases
for InboundShipments.
Now able to use all pytest features in this module.
This should set a standard for migrating tests for other requests
over to pytest.

Renamed "tests/apis/inboundshipments/test_requests" to
"tests/apis/test_inboundshipments", which should clean up
the directory structure there a bit and allow easier sharing
of common tools.

Got rid of that utils module, changed to common.

Slight adjustments in base MWS tests and small fry tests,
on account of changes fixtures.
test_calc_request_description really didn't need fixtures there at all,
as it just has to build a string: so now we set that string manually.
Meanwhile, MWS base test is adjusted to something similar to changes
already made in #222.
(intend to adjust that to match this, to avoid some conflicts later)
Copy link
Copy Markdown
Member

@Bobspadger Bobspadger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

===================================================================================================================== warnings summary ======================================================================================================================
tests/test_mws_base.py::test_mws_get_service_status_deprecated
  /Users/alex/GIT/OpenSource/python-amazon-mws/mws/utils/parsers.py:168: RemovedInPAM11Warning: DictWrapper is deprecated. For parsing 'request.Response' objects, use 'mws.utils.parsers.MWSResponse'. For parsing raw XML content, see module 'mws.utils.xml'.
    warnings.warn(

tests/test_mws_base.py::test_mws_get_service_status_deprecated
  /Users/alex/GIT/OpenSource/python-amazon-mws/mws/utils/parsers.py:96: RemovedInPAM11Warning: 'XML2Dict' is deprecated. XML parsing is now performed by dependency 'xmltodict', using 'xmltodict.parse' (See module 'mws.utils.xml' for details).
    warnings.warn(

tests/test_mws_base.py::test_mws_get_service_status_deprecated
tests/test_mws_base.py::test_mws_get_service_status_deprecated
tests/test_mws_base.py::test_mws_get_service_status_deprecated
tests/test_mws_base.py::test_mws_get_service_status_deprecated
tests/test_mws_base.py::test_mws_get_service_status_deprecated
tests/test_mws_base.py::test_mws_get_service_status_deprecated
tests/test_mws_base.py::test_mws_get_service_status_deprecated
  /Users/alex/GIT/OpenSource/python-amazon-mws/mws/utils/parsers.py:33: RemovedInPAM11Warning: 'ObjectDict' is deprecated. Use 'mws.utils.parsers.DotDict' instead.
    warnings.warn(

-- Docs: https://docs.pytest.org/en/stable/warnings.html
============================================================================================================== 395 passed, 9 warnings in 1.82s ==============================================================================================================

Copy link
Copy Markdown
Member

@Bobspadger Bobspadger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All seems sensible to me, the tests pass and raise the required warnings happily.

@GriceTurrble GriceTurrble merged commit 9591d40 into develop Nov 3, 2020
@GriceTurrble GriceTurrble deleted the feature-deprecated-arguments branch November 3, 2020 15:35
@GriceTurrble GriceTurrble added this to the 1.0dev16 milestone Nov 18, 2020
GriceTurrble added a commit that referenced this pull request Jan 14, 2021
…, tests, and documentation (#232)

* parse_item_args docstring cleaned up for rst import

* `InboundShipments.from_address` changed to property
Now performs its parsing logic when using the setter mode.
Old `set_ship_from_address` changed to alias method.
Will deprecate this later.

* InboundShipments API reference added to docs

* Model for inbound shipments address added
Added as a temp solution while waiting for merge of #220 to formalize
(will then move to appropriate location)
Tests updated. Lots of tests that should raise exceptions no longer do by design.

* [WIP] topic doc for managing FBA shipments
Addresses concerns like #217, but currently incomplete.

* [minor] Setup.py does not use sys: removed

* [minor] License file rename
similar to master incoming change

* Tools consolidation into setup.cfg
similar to master incoming changes
Removed TravisCI config
removed Makefile

* Move flat_param_dict requirement to a staticmethod
Provides easier access to that utility within datatype classes

* [minor] Typos in comments

* Docstrings added to products datatypes

* Type hinting add to flat_param_dict

* Types and docs added to _flatten staticmethod

* Reprs added to products models

* Address model moved to appropriate models location

* Ignore pythonenv folders (codespaces)

* [minor] Moved from_address_dict
Better grouping with other methods.

* Remove unneeded assert tests

* Typing and type checking corrections
Type hinting for all methods in InboundShipments class
Adjustments to isinstance checks,
now looking for generic Mapping and Iterable types
(note the renaming of IterableType and IterableAbc, to
avoid confusion between the collections.abc generic
and the typing colleciton)

* Adding from_address optional arg to shipment methods
Allows users to set the from_address on a per-request basis.
Tests modified to clear this option (coverage not up to par yet!)

* [wip] Doc topic for managing shipments updated

* Cleaning up models interface
Introduce the to_params method, part of the base class.
Request methods should call on to_params to return flat dict results
Internally, models require implementation of params_dict
to create params dict specific to the model.
Prefixes can be added by the prefix arg in `to_params`,
which will call flat_param_dict for completeness.

* [wip] FBA doc updated, matches code state

* [wip] TODO notes added for a new model to handle item parsing

* [wip] more docs added.
Need to replace item parsing with a model
for better handling in future
Docs need to reflect that change coming up.
For now, more details to be added on the shipment creation process.

* Add Ubuntu 20 to test suite
Not being tested directly,
and it would be worthwhile to know if it breaks there for some odd reason
(unlikely, but never hurts)

* Type hinting and isinstance checks fixing in params utils

* Add enum for CurrencyCode to Products modelsShould be able to output itself the same way as before.User can also skip this and use string literals.

* Add models for CreateInboundShipmentPlan items
Should be able to supply a sequence of InboundShipmentPlanRequestItem instances
to request method.
Adjustments to request method to accept this model forthcoming.
Main benefit: model can accept PrepDetails, unlike current arg parsing.

* Remove unnecessary tests of underlying tools

* InboundShipments tests cleaned up
Preparing to overhaul further by removing unittest entirely

* Completion of model for InboundShipmentPlanRequestItem
Able to handle inputs for CreateInboundShipmentPlan requests.

* [wip] Starter model for InboundShipmentItem
Datatype for CreateInboundShipment and UpdateInboundShipment
More to be filled in later.

* Auto reprs for MWSDataType subclasses.

Was really starting to overthink this one.
Instead of defining a repr on every subclass that essentially does the same thing,
instead opt for an auto-repr pattern
on the abstract base class (ABC), MWSDataType.

There aren't any options here for excluding arguments with defaults,
and it's possible that something could be included from the `__dict__`
that isn't really an attribute of the class.
Particularly if there's something stored in the class
by some name other than the `__init__` arg.

I think the simplicity of this change outweighs those downsides, though.
Besides, if someone uses a design pattern in which the repr doesn't match up,
that's a style issue more than a breaking change,
something we're more likely to deal with in a code review than at runtime.

* Docstring adjustments for docs output.

* Renamed function broke autodoc, corrected.

* [wip] More details in FBA shipment doc
Need to add more to the later section for sending a request, making this incomplete.
This is a good start, I think.

And I'm *pretty* sure I'm done making code edits for the sake of better documentation.
Need to stop here for now and work on testing for the changes made to the API thus far.

* Implement InboundShipmentItem model
The two models for shipment items - `InboundShipmentPlanRequestItem`
and the new `InboundShipmentItem` - are similar in structure,
with only minor differences in optional params and the specific name
of the parameter for "quantity" ("Quantity" vs "QuantityShipped").
So, this change moves the logic originally in
`InboundShipmentPlanRequestItem` to a base class,
`BaseInboundShipmentItem`; which is then subclassed by both of the
real data models.

Each of those classes now independently implements its optional args,
such as "asin", "condition", and "release_date"; as well as the
`param_dicts` method (required by abstract base class `MWSDataType`)

The main wrinkle was the quantity param, which has different naming
depending on which model is being used:

    "Quantity" for `InboundShipmentPlanRequestItem`
    "QuantityShipped" for `InboundShipmentItem`

This is handled by a class attr, `quantity_param`, which is injected
as a key in the param dicts within `_base_params_dict`. An assert
attempts to handle a case when someone is implementing the base
class (which they shouldn't), as we don't want to try setting
the empty string "" as a key in a dict, I think.

Further, the base class uses `_base_params_dict` instead of
implementing its own `params_dict` as an additional protection.
The abstract base class `MWSDataType` requires this method be
implemented on a subclass, otherwise it throws an error when
trying to instantiate that class. Thus, if the user attempts
to instantiate `BaseInboundShipmentItem`, they get an error; but if
they instantiate one of its subclasses, that method is defined properly,
and no error is thrown.

This should help drive user behavior to use the appropriate subclasses
instead of trying to use the base class in their own code. The only
way that can properly use it is to subclass it themselves, and at
that point we assume they know what they're doing.

* Shortened names of class attrs for PrepDetails.

Don't see a need to keep the "OWNER_" naming for these two,
as it just adds more text the user has to enter needlessly.
At that point, they may as well skip using the class attrs
and just remember "AMAZON" and "SELLER" magic strings in their code.

Also some minor adjustments in [wip] docs for managing FBA shipments,
including the change that mentions the class attrs for PrepDetails.

* [docs] Cleaned up reference linking

Don't really need to use all these :ref: tags all over the place,
and those ugly reference targets at the top of docs.
Now uses the `:doc:` directive and appropriate links.

* [minor] doc links changed to https

All these docs links to MWS documentation used http, despite all being
available as https.
Probably some legacy oversight.
Corrected.

* [docs] [mior] Renamed files to match class namesFigure it best to have the doc titles (which are part of the url)match the name conventions of the classes they reference.

* [docs] auto docs for Products API

Adds in the API reference as well as data models.

* [docs] Docstring updates for inbound models

Doc strings here now more detailed in one aspect,
and properly create links with titles in auto reference
(nicer to read than the full URL).

Also included the full set of values for ItemCondition,
so that those can show up in the docs, as well.

* [docs] [wip] Details added to FBA topic

Includes mention of the Address storage option,
adds an overview for full workflow,
changes some secton titles to be a bit clearer,
and starts describing the methodology for parsing
shipment plans from a response.

From here, going to keep adding content to finish the workflow,
to include the new InboundShipmentItem model.
(actually, a code update is needed to implement that!)

* Cleaned up item parsing logic and docstrings

Allowed parse_shipment_items to use the new item model,
which was relatively simple. A bit harder was cleaning up
all the docstrings for the two parser methods,
which were just a mess.

* Cleaned up doscstring and error message for from_address setter

* Simplified from_address handling in request methods

Moved that logic out of those request methods
to a dedicated method, from_address_params
(replaces from_address_dict).

This method takes care of all the logic necessary for generating the final set of params
for the from_address, whether that comes from the API storage
or the overriding `from_address` argument.

* [docs] Remove autodocs for parsing logic

Users don't really need to see that stuff, methinks.

We'll add some discussion on the legacy item structure to the FBA topic,
but I don't think including this reference really added much,
and probably confused more than needed.

* [docs] Request method links now RST links

Updated the links to Amazon MWS documentation
for each request method in InboundShipments API
so that it is a properly-formatted external link
in reStructuredText.

This way, the auto-documentation for the class
will display these links more cleanly.

I could have opted to put this kind of change into docs themselves
and leave the systems separate, but overall I think
it's easier to maintain the reference within the code base,
and let the docs display that.

* [docs] Cleaned up docstring for init

Did not properly describe what's actually happening here.
This concise version is, I think, much clearer as to its intent.

* Added docstring for a deprecation note

Not raising a warning from this method just yet (considering it),
but noting that this old interface using a setter pattern
is now just an alias for the property setter for from_address.

Going to want to remove that eventually, I think. Or just get rid of
the address storage logic entirely,
though it is rather convenient.

Anyway, this is just noting that it's not advisable to use this method.
User should just assign to from_address directly.

* [tests] Testing overhaul on InboundShipments

Now properly using pytext and fixtures to fullest extent
across the InboundShipments API class.

Fixtures for "cred_X_Y" removed, as they were really not needed.
Replaced with constants that I can access within non-test methods,
such as common_assert_params.
Other methods that might have used those fixtures
can instead use `mws_credentials` and remove options as needed.

An api_instance fixture is included, which can read request context
and take an api class from the calling class.
This requires it be used in classes that set `api_class`,
but that's a minor concern.

Common tooling for the newer tests is similar to old, with tweaks to work
with pytest fixtures more cleanly.
No more setup logic for self.api, hello api_instance fixture in
common tests!

All reliance on unittest removed from the test cases
for InboundShipments.
Now able to use all pytest features in this module.
This should set a standard for migrating tests for other requests
over to pytest.

Renamed "tests/apis/inboundshipments/test_requests" to
"tests/apis/test_inboundshipments", which should clean up
the directory structure there a bit and allow easier sharing
of common tools.

Got rid of that utils module, changed to common.

Slight adjustments in base MWS tests and small fry tests,
on account of changes fixtures.
test_calc_request_description really didn't need fixtures there at all,
as it just has to build a string: so now we set that string manually.
Meanwhile, MWS base test is adjusted to something similar to changes
already made in #222.
(intend to adjust that to match this, to avoid some conflicts later)

* [docs] Remove products reference

This is not in scope for current set of changes,
so I don't want to hinder progress by including it now.

Will bring this back in a future update that focuses more on docs
for the general API reference.

* [bug] Should not put self in a super call.

Force of habit to include the self as a function def,
but should not be included as first argument,
else it ends up in some weird behavior.

Whoops!

* Permitted operations check for models.

The base datatype, MWSDataType, now includes `operations_permitted`,
a class-level list of strings that should contain the Action names
for MWS operations in which the given model may be used; and
`raise_for_operation_mismatch`, a method that checks an operation name
against that list of permitted operations. The expected convention is
to use the MWS operation's Action name, i.e. "SubmitFeed", as these
names are something we cannot alter (MWS expects those actions;
we can change the Python method names any time we want, but the
Action names should remain the same).

This is designed as an opt-in mechanism that must be implemented
in two places to take effect:

1. The model subclass must add an `operations_permitted` class with
   a list of Actions permitted for that model; and
2. The request method using that model must manually call
   `raise_for_operation_mismatch` and provide the `operation` name.

With this design, models `InboundShipmentPlanRequestItem` and
`InboundShipmentItem` implement this mechanism with definitions for
the operations permitted and the `raise_` method called within
`parse_shipment_items`. This way, if users attempt to use the
incorrect model in the CreateInboundShipmentPlan, CreateInboundShipment,
or UpdateInboundShipment operations, they will get an MWSError
to indicate the problem.

Initially I thought about adding this mechanism solely to
`BaseInboundShipmentItem`, so that those downstream models would have
that mechanism available. The more I thought about it, though,
it became clear that giving that utility to any model in the system
was a better use of that effort. Probably YAGNI for other model designs,
but the implementation was simple enough. Plus, being able to rely on
`operations_permitted` as a class attr of all datatype models *and*
having ready access to the `raise_` method without needing to write
a new one somewhere else in the module was quite helpful.

Additional changes:

- updated docstring for `parse_shipment_items` and `parse_legacy_item`
  functions.
- defined a Literal type, `ITEM_OP_TYPE`, to use in the annotations
  for those functions, hopefully to make it more clear what those
  functions are expecting.

* [bug] Python<3.8 does not have Literal type

Attempting to provide the type hinting for operation as a literal,
but doing so with the TYPE_CHECKING flag and a commented type,
which should be ignored at runtime.

This ought to correct issues where Python<3.8 cannot use the Literal
type, but in development on 3.8+, it should provide good hints
to the IDE.

* [todo] note added for Literal pre-3.8

* [tests] expanded tests for FBA shipment methods

Checks:

- CreateInboundShipmentPlan
- CreateInboundShipment
- UpdateInboundShipment

For each method, checks:

- no items passed (exception for some, pass for others)
- no address saved, exception raised
- legacy item dicts used, params match
- item models used, params match
- wrong item model used, exception raised

Should complete the basic testing requirements for the new item models
for these operations.

* Remove dev update from README, move to new CHANGELOG

Also includes TODO for filling in the 1.0dev16 notes, to be added shortly

* Bump verisons to 1.0dev16

* [wip] Changelog for 1.0dev16 in docs

Stub added for now, filling in later.

* Remove unused import commented out

* Fix breaking change from merge

* [tests] Usage of from_address_params method.

* [bug] Using IterableAbc here didn't actually work.

Changed back to old way, testing for type list, set, or tuple.
This is more in line with what's expected, and avoids passing single string
instances to the enumeration.

* Ignore typing branch in coverage

* [tests] Coverage for parse_legacy_items errors

Small test overall, and it's for a minor piece of the tooling.
But, I figure ensuring this works as expected can help with
some potential legacy bugs in future.

* [tests] Minor test coverage for errors in parse_shipment_items

* [docs] [minor] Broken link to prereqs doc corrected

* Special constructor for inbound shipment item

Able to build an item of this model from a plan item DotDict instance.
Should aid in getting folks to inject the plan details for an item
so that they can use CreateInboundShipment
in a more streamlined setting.

Docs update to follow.

* Constants for shipping statuses; create_ default status WORKING

Kind of a missed opportunity there, not supplying useful values
for shipment status. Also setting the default to WORKING
helps newcomers create their working shipments
without forgetting anything.

* Added constants for box_contents_source arg

* Renamed to `from_plan_item`

Think this naming makes more sense in context.

* Method for bulk item processing from a shipment plan.

`shipment_items_from_plan` can be used to process the contents of
`resp.parsed.InboundShipmentPlans.member` (typically part of a response
from a `InboundShipments.create_inbound_shipment_plan` request).
This converts the `Items` found in response content to a list of
`InboundShipmentItem` instances, ready to be passed to
`InboundShipments.create_inbound_shipment`.

To account for the lack of data for case-packs and pre-order item
release dates, function accepts the `overrides` argument, which expects
a dict of SKUs with either an `ExtraItemData` instance or another
dict with keys "quantity_in_case" and "release_date" (matching the
signature for the `InboundShipmentItem` constructor and the
`from_plan_item` alternate constructor).

With a set of overrides provided, SKUs in the planned shipment are
matched up, with the details from `overrides` injected into the new
`InboundShipmentItem` instance.

Tests have been added to cover the use cases for this function,
with some adjustment to fixtures to streamline that test code a bit.

Using this, a user can now gather items from a shipment plan
very simply like so:

    from mws.models.inbound_shipments import shipment_items_from_plan
    from mws.models.inbound_shipments import ExtraItemData

    # overrides can be constructed ahead of time to inject
    # case-pack and release date info.
    # No SKUs can be added to a shipment this way.
    # If using a standard dict, any keys not matching
    # quantity_in_case or release_date are ignored.
    overrides = {'mySku': ExtraItemData(...), ...}

    for plan in resp.parsed.InboundShipmentPlans.member:
        shipment_items = shipment_items_from_plan(
            plan, overrides=overrides,
        )
        inbound_api.create_inbound_shipment(
            ...,
            items=shipment_items,
            ...,
        )

More details to provided in a docs update coming shortly.

* [docs] Overhaul on FBA shipment creation docs.

Takes us up to completion of creating an FBA shipment, including
different methods for gathering items from responses to
`create_inbound_shipment_plan`.

Beyond this content, may still want to add some details on updating
a shipment, shipment status, getting package labels, etc.
All of that is relevant, but remains a deep topic, and adding all that
to the same doc may be a bit much (already at 500+ lines here).

* Keep forgetting I can't use dataclass in <3.6

* Adjustments to classifiers in setup.py

* Remov unneeded import

* Fill out the docstring with details on overrides

* Changelogs updated

Includes descriptive links to the issues related to dev16 update.

Milestone for 1.0dev16 added to project to serve as a link target.
In future, we'll create milestones for each of these versions
so that we can link to the past issues in the same way.
This should aid in clarity on the development process moving forward.
(Borrowed this idea from the release notes for VS Code;
learn from the best, eh?)

* Code cleanups

Reorganized imports across these modules using isort
(or rather VS Code command "Python Refactor: Sort Imports",
which calls on isort at the lower level)

Exploded argument lists in functions to multiple lines to aid in
readability. With some more complex type annotations in use, having
the arguments on multiple lines, even for a short set of args, makes
it a lot easier to decipher.

Changed some type hints from `Iterable` to `List`. No underlying code
change, but I think indicating the `List` type is easier than specifying
the more generic `Iterable`.

Added `utils.params.iterable_param` function. Replaces instances that
would follow this pattern:

    if not isinstance(val, (list, set, tuple):
        val = [val]

I just think it makes more sense to codify that usage under its own
utility, otherwise the code base is not very DRY.
Actual code is a bit more generic than previous examples, but boils down
to wrapping non-iterables in a list, with the exception that strings
are not counted as "iterables".

Some type annotations for request method arguments in InboundShipments
have been updated, as well as some docstrings that were incorrect.
More scrutiny needed here in near future, I think.

Moved back from the pattern of condensing `make_request` calls to single
lines if using a small set of data: now standardized on the pattern:

    data = {}
    return # request goes here.

Further expanded some other calls to utility methods, specifying the
argument names explicitly. This expands the number of code lines in
the module, sure, but leaves it more verbose and easier to read.

* Elevate Creating Shipments header; remove optional args from example.

* Correction, supporting 3.6+; no longer testing 3.5

* Set all update args to optional (besides shipment_id)

This may not necessarily be feasible for MWS requirements, but I cannot
think of the reason to leave these as required without a documented
reason to require them.
Therefore, when and if someone responds with an issue documenting
why some parts of this are *not* optional, then we can adjust it later.

* [wip] initial section for updating shipments

* Additional details for updating shipments

At this stage, I am exhausted of this documentation.
I'm sure there's more that can be written, but I can't think of it
right now, and it's time to move on to other concerns instead of
holding this up and further.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants