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

Feature implement get my fees estimate#220

Merged
GriceTurrble merged 18 commits intopython-amazon-mws:developfrom
adspert:feature-implement-get-my-fees-estimate
Oct 9, 2020
Merged

Feature implement get my fees estimate#220
GriceTurrble merged 18 commits intopython-amazon-mws:developfrom
adspert:feature-implement-get-my-fees-estimate

Conversation

@mdaif
Copy link
Copy Markdown
Contributor

@mdaif mdaif commented Oct 5, 2020

This PR aims to implement get_my_fees_estimate

The tricky part about this method is to:

  1. Reuse the existing utils in a DRY manner.
  2. Enforcing the method's client to structure the input data into a certain (pretty complicated) format.

Following some best practices from Domain Driven Design, I used classes to represent the value objects (such as ListingPrice, ShippingPrice, .. etc). This allows the client to construct the complicated request in a simple manner.
I also made use of Python3 type annotations to clarify the data types.

I've also included a test case to elaborate how to use the method.

@codecov
Copy link
Copy Markdown

codecov bot commented Oct 5, 2020

Codecov Report

Merging #220 into develop will decrease coverage by 0.00%.
The diff coverage is 81.35%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #220      +/-   ##
===========================================
- Coverage    81.45%   81.45%   -0.01%     
===========================================
  Files           28       30       +2     
  Lines         1278     1337      +59     
  Branches       141      144       +3     
===========================================
+ Hits          1041     1089      +48     
- Misses         211      220       +9     
- Partials        26       28       +2     
Impacted Files Coverage Δ
mws/utils/params.py 95.69% <ø> (ø)
mws/models/products.py 78.72% <78.72%> (ø)
mws/models/base.py 80.00% <80.00%> (ø)
mws/apis/products.py 93.75% <100.00%> (+0.76%) ⬆️

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 0d93238...fb90124. Read the comment docs.

@GriceTurrble
Copy link
Copy Markdown
Member

@mdaif Thank you for the contribution. Please note that a major update has been posted to develop branch (in the works for some time now), so you may want to sync up there first before we come back for a review.

Among the changes:

  • The utils.py module was removed, in favor of a utils directory with different modules tied to specific concerns. If you have utilities to add, you will want to find the right home for them.
    • I don't see your previous changes to utilities in the changed files currently, so not sure if that got removed or if you changed it already.
  • Request method signatures are changing slightly. See examples in newer code, where action positional arg is required instead of an 'Action' key in the data dict.

Other than that, the changes look good so far. I'll come back for a more thorough review once you've had time to check the upstream changes. Give me a poke when ready. :)

Mohamed Daif added 3 commits October 7, 2020 10:01
…implement-get-my-fees-estimate

# Conflicts:
#	mws/apis/products.py
#	tests/request_methods/test_products.py
@mdaif
Copy link
Copy Markdown
Contributor Author

mdaif commented Oct 7, 2020

@GriceTurrble Thanks a lot for your timely review and comments ! :))
I've addressed your comments and extended the test case.
There's however a missing style check but I don't know what's failing exactly, I ran flake8 on my modules and it's passing.

@GriceTurrble
Copy link
Copy Markdown
Member

The failing check uses Black for code formatting. Simply pip install black in your virtual env and then black . on the root directory of the repo, and the code will be reformatted automatically.

Occasionally this can introduce new linting errors, so please double-check flake8 output after formatting.

@mdaif
Copy link
Copy Markdown
Contributor Author

mdaif commented Oct 7, 2020

Hi @GriceTurrble
Thanks for the tip, will do that. A quick note though, it seems like the support for Python 3.5 is still required although black is a 3.6+ package and Python 3.5 reached the end of its life on September 13th, 2020. Will support for Python 3.5 be dropped soon ? (I know about the new SP API that'll deprecate MWS anyhow, but that'll probably take quite some time)

@GriceTurrble
Copy link
Copy Markdown
Member

As a matter of fact, I dropped 3.5 from the test suite on dev just prior to the update this week. So no, 3.5 support is not required. :)

@mdaif
Copy link
Copy Markdown
Contributor Author

mdaif commented Oct 7, 2020

Great :'D ... I've also just noticed that ubuntu-latest,3.5 check is not there anymore, but it's still in .travis.yml file, so to be on the safe side my code still supported that.

@mdaif
Copy link
Copy Markdown
Contributor Author

mdaif commented Oct 7, 2020

I've tried to include dataclasses but the different python versions made that difficult. Anyhow, I think we're ready for the second round of reviews :))

@GriceTurrble
Copy link
Copy Markdown
Member

Ah. I'm ignoring the travis settings myself, so no big. If it covers 3.5 support, all the better, but we just won't be running tests against it.

Galen Rice and others added 6 commits October 7, 2020 16:41
Sets up base MWSDataType that all models should subclass
Method to_dict must be implemented on subclasses.
Names of datatypes altered to match those in MWS docs
Points datatype added
Each is able to flatten related sub-types (like ListingPrice and Shipping)
Tests updated to match changes
Utilize `ABCMeta` and `abstractmethod` to indicate the method must be implemented on child methods.
(writing from UI, will have to check test output from CI workflows to see if this works or revert it)
Not really a need for that complication, simplify it.
Now includes a required positional arg as well as the optional `*args` style for additional estimates.
All are joined to the same list as before.
GriceTurrble added a commit that referenced this pull request Oct 9, 2020
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.
@mdaif
Copy link
Copy Markdown
Contributor Author

mdaif commented Oct 9, 2020

I merged too early, there were a couple of failing checks. I'll have a look at that.

@mdaif
Copy link
Copy Markdown
Contributor Author

mdaif commented Oct 9, 2020

Fixed now.

Copy link
Copy Markdown
Member

@GriceTurrble GriceTurrble left a comment

Choose a reason for hiding this comment

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

A nice joint effort at the end there. Thank you for incorporating my suggested changes, and for checking my work to make sure things work properly. 👍

This adds in the get_my_fees_estimate, as the PR states, but also introduces a good framework for us to add datatype models that are closer in structure to the ones used by MWS, allowing us to offload some data validation and serialization concerns to those models. Adds a bit of complication, of course, but sets a great baseline and provides those tools to other developers to hopefully make their lives a little easier.

I hope to build off what this change adds as we add coverage for the remainder of MWS in the near future. Thank you kindly!

@GriceTurrble GriceTurrble merged commit fba2c9c into python-amazon-mws:develop Oct 9, 2020
@mdaif
Copy link
Copy Markdown
Contributor Author

mdaif commented Oct 9, 2020

That was awesome ! Thank you ! :))

@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.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants