Skip to content

Fix for user_paths.py errors#76

Open
JadenStrom wants to merge 1 commit intolasp:masterfrom
JadenStrom:fix-user-paths
Open

Fix for user_paths.py errors#76
JadenStrom wants to merge 1 commit intolasp:masterfrom
JadenStrom:fix-user-paths

Conversation

@JadenStrom
Copy link

@JadenStrom JadenStrom commented Jun 9, 2025

Pull Request Checklist

  • Code Follows PEP8 guidelines (explain any exceptions)
  • New routines don't duplicate existing functionality
  • Original authors of modified routines have been added as reviewers

This requests fixes issue #74


# Get orbit folder
orbfold = orbit_folder(iuvs_orbno_from_fname(l1a_fname))
from maven_iuvs.download import get_default_data_directory
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe imports are required to be at the top of the file or it will throw an error

Copy link
Collaborator

Choose a reason for hiding this comment

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

It needs to be here to avoid a circular import error, but this should be explicitly noted with a comment rather than implied.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, you can also import the download.py module as an alias, like:

# miscellaneous.py:

import maven_iuvs.download as dl
...
blah = dl.get_default_data_directory('l1a')

But this is not the most elegant since it imports the whole module. At any rate, I have something like this in echelle.py and echelle_graphics.py to avoid the same problem, but that's probably better addressed by eventually splitting up the quicklook vs. line fitting codes.

Copy link
Collaborator

Choose a reason for hiding this comment

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

To clarify, @JadenStrom please add either a comment to explain the positioning, or use the tactic I showed above. Make a new commit, push to origin, and it should show up here.

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.

3 participants