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

Question : is code used to follow relationships needed for imports ? #82

Closed
pstch opened this issue May 1, 2014 · 3 comments
Closed

Comments

@pstch
Copy link

pstch commented May 1, 2014

Hi,

I'm having a hard time understanding the code in resources.py, especially these lines. I think I got my head around it, and I just want to confirm something.

Is the whole bunch of lines from 431-455 just dedicated to export ? What happens if ResourceOptions.fields contains a field that folllows a relationship (ex joint_venture__company__name, and we import some data ? Would the name attribute (of company) get updated for each row ?


Also (don't read if you're in a hurry):

I recently started to integrate django-import-export in a big object management application that I'm writing for a client, and to make import/export functionality available in my generic views (with support for filtered exports as it's already available for the admin, dynamically sorted exports, and on-the-fly import resource definition). In order to do that, I need to understand django-import-export's code, and I started to refactor it a bit. I'll be happy to submit my changes, however, sadly I made a lot of coding style changes, and I know that many developers really don't like that. Sorry :(. Anyway, here is a list of things I did, it'd be very kind of you to me tell if you see anything in that list that you really dont want to merge :

  • Line-length: I try to stick with the standard of 79 chars in each line.
  • Blank lines: I use many blank lines in long functions, makes the line length bigger but it think it's better for readibility
  • Long statement folding: I tend to fold long lines a lot, and I usually put each argument in its line when indent level is high, or argument names are long
  • Commenting: I comment a lot, especially when refactoring. I'll try to clean them up at the end
  • Docstrings: I start docstrings just after the opening triple-quote, and always leave an empty line at the end (PEP-style). I try to document every function, especially when refactoring.
  • Code splitting: I split code in small functions, maybe too much. Also, when using Django internals (such as _meta), I tend to access them only with functions defined in a separate file, so that if there's any change upstream (as _meta retro compatibility is not guaranteed at all)
  • Functional builtins: I prefer using for item in filter(lambda x: test, list): <code> rather than for item in list: if test: <code>, for example. Tell me if that annoys you, I can easily revert this.
  • Generators: I also use them frequently, especially when dealing with big datasets.
  • Var names: I hate f ! I prefer having longer, explicit names than short, easy-to-type names.
  • Function names: For example, I changed the name of Resource.get_export_headers, as is it not specific to export (it is called for each line when importing data)
@bmihelac
Copy link
Member

bmihelac commented May 2, 2014

Hi @pstch! Regarding code in resources. py it is only used for export and will not affect importing - this should be similar how Django and tastypie work.

Regarding refactoring I would appreciate and merge back everything that improves documentation or pep cleanup. I try hard not to break backward compatibility and not to introduce too specific features that would make code harder to maintain.

@bmihelac
Copy link
Member

bmihelac commented May 2, 2014

also smaller pull requests makes it much easier to review and merge back.

@pstch
Copy link
Author

pstch commented May 2, 2014

Ok great :) I do not plan on integrating any new real feature at the moment, everything specific stays in the actual application code.

Except for one thing, though : I think that having an example of a generic (class based) view would be nice, it would be easy to test, and would provide something easy to subclass for django-import-export users. It would also be rather simple, as everything is already done by ImportMixin. What is your opinion on this ?

Noted about the smaller PRs

@pstch pstch closed this as completed May 2, 2014
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

No branches or pull requests

2 participants